25 lines
3.1 KiB
Markdown
25 lines
3.1 KiB
Markdown
# Focused diagnostic feedback review
|
|
|
|
Executor resolution (2026-10-15): structured static failure causes and numeric HTTP/WebView status are now recorded; graduate probe exceptions produce a type-only breadcrumb. Login diagnostic regression was observed RED→GREEN; final Flutter suite249/249 and backend54/54. Original independent report follows.
|
|
|
|
Scope: new diagnostic logging and feedback multipart changes only, compared with HEAD. Undergraduate final-review fixes and glass changes were excluded. Read-only inspection; no deployment or private page content accessed. Existing test additions inspected; no redundant test run while the implementer is running the feature suite.
|
|
|
|
## Important finding
|
|
|
|
[P2] Preserve a structured reason for terminal school-read failures.
|
|
|
|
Location: `mobile/lib/features/school/school_session.dart:272-278` (and equivalent `mobile/lib/features/school/undergraduate/session.dart:219-225`). All callers currently produce `school_read_failed` with the same `code: flow_failure`. Minimal trigger: let the graduate main-frame HTTP request fail versus let the same target time out; both logs contain the identical failure event even though the UI already distinguishes those causes. A 403/500, a main-frame WebView resource error, a rejected navigation and a parsing/read timeout cannot be separated in the feedback attachment. This materially limits the stated import-stall diagnostic purpose. Pass bounded static reason codes through `_failure`, plus numeric HTTP/WebView error codes where applicable, without recording URLs or exception messages.
|
|
|
|
Closely related coverage gap: `SchoolSession._loaded` still silently swallows the outer JavaScript/evidence exception (`school_session.dart:497-499`). If `runJavaScriptReturningResult` repeatedly throws while the term selection flow is waiting, `finding` remains false and `_scheduleProbe` can exhaust its retries without a `school_evidence`, parse-rejected, or terminal-failure breadcrumb. Record a bounded probe-failed event (exception type only) in that catch so the attached log identifies this stall path.
|
|
|
|
## Other areas checked
|
|
|
|
- Checkbox starts unchecked; logs are sent only in the opted-in problem submission.
|
|
- Live callers never pass credentials, cookie values, raw URLs/HTML, feedback text or course content. URL classification emits only a fixed page category.
|
|
- Cache of `_pendingSubmission` preserves immutable log bytes and the same submission ID across retry/status checking.
|
|
- Media validation remains independently limited to three files; backend allows one extra diagnostic part and limits it to 256 KiB.
|
|
- Normal generated entries are ASCII and limited to 500 rows with bounded event/field strings, so the exported payload stays below 256 KiB.
|
|
- Startup attaches persistence without awaiting it; logger file failures are swallowed and cannot fail app startup.
|
|
|
|
Non-blocking observation: persistence rewrites the destination directly on each accepted event, so termination during a write can leave a truncated log and lose the previous window. Atomic temporary-file replacement would improve crash resilience, but I have not treated this as a release blocker absent an explicit crash-durability guarantee.
|