fix(llc): reconnect on a server-side processing timeout, and stop parsing errors twice

[xsahil03x]

Three of the five review findings on #168. The two left out are deliberate: see the end.

1. A 408 refused to reconnect, contradicting the documented contract

ERROR_LAYER.md says 408 is "a server-side processing timeout, not a verdict on the request" and retries with backoff. isAutomaticReconnectionEnabled fell through to the 4xx rule and refused it:

StreamApiException(:final statusCode) => statusCode < 400 || statusCode >= 500,   // 408 → false

Verified by running it, not just reading:

status 408 -> false      ← the doc says retry
status 429 -> true
status 500 -> true
status 403 -> false

Fixed in the code rather than by documenting the divergence, so the doc keeps stating one rule and the code stops disagreeing.

Risk: none against this backend. A 408 cannot reach the client over a socket. Every WS error frame comes from Conn.CloseWithError (monolith/server/ws/server.go:225, monolith/engine/websocket/conn.go:429) — the only two producers of NewConnectionErrorEvent — and its three call sites in monolith/server/base.go produce a ReadAuthMessage failure (400-class, :1660), an auth error (401/403-class, :1699) or a rate limit (429, :1760/:1783). All five producers of RequestTimeoutError are HTTP request-body or upload paths. So the new branch is inert today; it exists so a reader reconciling doc against code doesn't have to discover that to make sense of it.

Pinned by a test that fails when the branch is removed.

2. _ => true hid a non-exhaustive switch

ServerInitiated's inner switch matched all four StreamException kinds and then ended in _ => true, reachable only for a null error. Written as null => true, adding a fifth kind is a compile error instead of a silent default-to-reconnect. No behaviour change.

3. Every error response was parsed twice

AuthInterceptor.onError called toStreamException() — which runs StreamApiError.fromJson on the body — to find the one code it acts on, discarded it for everything else, then handed the raw failure to ApiErrorInterceptor, documented as installed last, which read the same body again.

Gating on statusCode would have been the obvious fix and is wrong: the layer's own rule is that statusCode and code are independent and neither may be inferred from the other. Instead it forwards what it already has — and the machinery was already there, since ApiErrorInterceptor short-circuits on StreamDioException and toStreamException() does too. The forwarded exception is the same object ApiErrorInterceptor would have constructed, so this is behaviour-preserving by construction.

Deliberately not in this PR

• The token-blip design question. A TokenProvider that throws its own error type on a transient failure gets wrapped into StreamAuthenticationException and is then non-reconnectable — on the exact path the SDK reconnects to fix. That changes runtime behaviour on a common path and is the PR author's call. Worth noting for whoever decides: DefaultRetryStrategy has no attempt cap, so "retriable" means unbounded retries at ≤25s intervals — which is already what SystemInitiated, UnHealthyConnection, ConnectTimeout and network-caused AuthenticationFailed do. Whichever way it goes, the missing test is the app-supplied-error case; the two Stream-typed cases are already pinned.
• Retry-After HTTP-date parsing. Nothing to do. The backend has one emission site, monolith/budget/middleware.go:399, strconv.FormatInt, and the runbook confirms "only on 429". There is no date form to drop.

715 tests pass, dart analyze and dart format clean.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a300c23-4307-4d7e-af28-d900dc8f5c5f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[xsahil03x]

Landed directly on feat/error-layer as 1aed17b rather than merging this PR, so #168 carries the fixes in its own history. GitHub picked that up and marked this merged — nothing further needed here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.21%. Comparing base (59adec5) to head (1aed17b).
⚠️ Report is 2 commits behind head on feat/error-layer.

Additional details and impacted files

@@                 Coverage Diff                  @@
##           feat/error-layer     #172      +/-   ##
====================================================
+ Coverage             66.18%   66.21%   +0.03%     
====================================================
  Files                   205      205              
  Lines                  8306     8315       +9     
====================================================
+ Hits                   5497     5506       +9     
  Misses                 2809     2809              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.