DX-1209: inline fix-it hints on SDK ErrorInfo throw sites#2233
Draft
umair-ably wants to merge 2 commits into
Draft
DX-1209: inline fix-it hints on SDK ErrorInfo throw sites#2233umair-ably wants to merge 2 commits into
umair-ably wants to merge 2 commits into
Conversation
Adopts the message-vs-hint split DX-1204 introduced (`err.message` says *what* failed; `err.hint` says *how* to fix it) and applies it at every ErrorInfo throw in the SDK where a fix-it hint adds value. Server-relayed codes (40140, 40142, 40160, 40300, 42910, …) are deliberately out of scope here — that propagation is being tracked separately. Covered (~80 sites across 19 files): - realtimechannel.ts — channel options validation, publish shape (40013), max size (40009), detach-while-failed, invalidStateError, release-state, attach/detach timeout, untilAttach, sendUpdate missing serial, sync not-attached. - realtimepresence.ts — clientId missing for enter/update/leave, leave-in-incompatible-state, get() on suspended, untilAttach, auto- re-enter failure. - auth.ts — no auth options, incompatible key (×2), authUrl/authCallback errors, no-renewable-token, no/invalid key, empty/wildcard/non-string clientId, token clientId mismatch, token-shape rejection. - baseclient.ts — invalid key format, clientId type/wildcard. - defaults.ts — endpoint/environment/host conflicts, host validation. - rest.ts — unsupported HTTP method, revoke-tokens-under-token-auth. - baserealtime.ts — channels.get with mode change after first call. - paginatedresource.ts — no link to first/current page. - utils.ts — derived-channel regex, missing-plugin, concurrent iterator. - connectionmanager.ts — ping when not connected, authUrl/authCallback 403/error. - basemessage.ts — unsupported data type, vcdiff plugin missing/decode/ unsupported. - push.ts / pushactivation.ts / pushchannel.ts / getW3CDeviceDetails.ts — push platform/state, registration callback, deviceId/device-identity preconditions. - restchannel.ts / restchannelmixin.ts / restannotations.ts / realtimeannotations.ts — message-serial requirements, annotation shape/mode. Skipped per `LLMEvalUpdatedPlan.md` A1 scope: - LiveObjects 92xxx (messages already specific). - Transport 80xxx lifecycle codes (auto-recovered; a hint would encourage retry code that fights the SDK). - 50000 internal errors (only actionable advice is file-an-issue, not worth a per-site hint). Tests pin the hint strings so silent rephrasing flags as drift, modelled on the DX-1204 v1-callback assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Sharpens ~15 of the inline error hints DX-1209 introduced based on ~120 LLM-pilot runs (see PR description for the full evaluation). Language: remove soft hedges (`probably`, `likely`, `potentially`, `may trigger`) in favour of active, definitive guidance. Second-wall forecasts: where a client-side fix opens up a server-side rejection, the hint now names the downstream cause inline. Applied to annotation_subscribe (realtimeannotations.ts), object_subscribe (liveobjects/realtimeobject.ts), wildcard clientId (auth.ts, baseclient.ts), presence.enter/update/leave (realtimepresence.ts), invalid channel modes (realtimechannel.ts), publish wrong-shape (realtimechannel.ts, restchannel.ts), and message annotations dashboard config (restchannelmixin.ts, realtimechannel.ts). The pattern brings LiveObjects 92xxx into scope — pilots showed weaker-model abandon rates of 40% on this surface without the forecast, 0% with it. Anti-hack notes: annotation_subscribe and object_subscribe hints now say "appending to channel.modes after attach() does not enable the mode server-side" so agents stop mutating the local modes array. Ably CLI pointers: where a CLI command would close the diagnosis loop, the hint adds a conditional `If you have the Ably CLI installed, ...` sentence (e.g. `ably auth keys list` for capability errors, `ably apps rules list` for channel-namespace settings). The conditional phrasing avoids regressing agents on machines without the CLI — an imperative phrasing caused 3/5 Opus runs to time out trying to invoke a missing binary in a confirmation pilot. Push platform-not-supported hints (push.ts ×2, pushactivation.ts ×4) now lead with the structural impossibility — "push.activate() cannot succeed in Node.js/server contexts (there is no device to register)" — and name the specific admin APIs to use instead. This turned 22-30-tool-call monkey-patch attempts into clean 7-tool-call acknowledgements that the SDK is right and the call is misused. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
message(what failed) /hint(how to fix) split that DX-1204 introduced fordetectV1Callbackand applies it to everyErrorInfothrow in the SDK where a fix-it hint adds value. The split lets agents (and humans) read the what inerr.messageand act on the how inerr.hintwithout parsing prose.Themes / where this sits
This is the foundation slice of
LLMEvalUpdatedPlan.mdA1 (TKT-6) and aligns with the team's "Theme 3 — errors guide self-healing" target for the quarter. It's defence-in-depth alongside DX-1204 (v1-callback runtime throws with a hint) and DX-1205 (legacy-import shims with a hint).What's in scope vs deliberately not
Hint added (~80 sites):
realtimechannel.ts— channel options validation, publish shape (40013), max size (40009), detach-while-failed,invalidStateError(90001), release-state, attach/detach timeout (90007), untilAttach,sendUpdatemissing serial (40003),sync()not-attached.realtimepresence.ts— clientId missing for enter/update/leave (40012), leave-in-incompatible-state (90001),get()on suspended (91005), untilAttach, auto-re-enter failure (91004).auth.ts— no auth options (40160), incompatible key (40102 ×2), authUrl/authCallback errors (40170 ×9), no-renewable-token (40171), no/invalid key (40101 ×2), empty/wildcard/non-string clientId (40012 ×3), token clientId mismatch (RSA15c), token-shape rejection.baseclient.ts,defaults.ts,rest.ts,baserealtime.ts,paginatedresource.ts,utils.ts,connectionmanager.ts,basemessage.ts— see commit message for the per-file rundown.push.ts,pushactivation.ts,pushchannel.ts,getW3CDeviceDetails.ts— push platform/state, registration callback, device-identity preconditions.restchannel.ts,restchannelmixin.ts,restannotations.ts,realtimeannotations.ts— message-serial requirements, annotation shape / mode.Skipped (per A1 scope):
92xxx— messages are already specific; adding a hint is low value.80xxxconnection-lifecycle codes — auto-recovered by the SDK; a hint would encourage user retry code that fights the SDK.50000internal errors — only actionable advice is file an issue, not worth a per-site hint.40140family,40160,40300,42910, etc.) — the SDK does not construct these, the server does. Propagating server-supplied hints is separate ongoing work.Accuracy verification
Hints were verified against the actual code at each throw site (not just the docs) — flagged and fixed issues during review:
MAX_TOKEN_LENGTH = 2^17≈ 128 KB (not the 384 KB the first draft claimed).@ably/vcdiff-decoder(not the legacy fork URL still referenced in the error string).auth.authorize()rejects any key change, not just cross-app keys.annotation_subscribe(mode) vsannotation-subscribe(token capability) — both forms are real and the hint uses each correctly.Out of scope but flagged
While reviewing throw sites I noticed four pre-existing
ErrorInfoconstructions withcodeandstatusCodeswapped (signature is(message, code, statusCode)):src/plugins/push/getW3CDeviceDetails.ts:31, :40—new ErrorInfo('…', 400, 40000)src/common/lib/util/utils.ts:472, :479—new ErrorInfo('…', 400, 40010)These pre-date DX-1204 and aren't touched here. Worth a separate ticket — the hint additions in this PR are stamped onto these malformed
ErrorInfos in those files.Base branch
DX-1205/legacy-import-shims, notmain. We're stacking — merge DX-1205 (#2227) first, then this branch auto-retargets tomainor can be rebased.Test plan
npx mocha test/unit/*.test.js— 23 passing (existing + newtest/unit/error-hints.test.js).npx tsc --noEmit— clean.npm run build:node— clean.test/unit/error-hints.test.jspins exact hint strings for the construction-time inline throws that don't require a live connection (invalid key, wildcard clientId, no auth options, endpoint/environment conflict, missing-plugin). Drift fails the test.🤖 Generated with Claude Code