Split overloaded error codes 104001-104003 - #288
Open
lmars wants to merge 2 commits into
Open
Conversation
Three of the SDK's error codes each covered several unrelated failures, so a developer could not switch on the code and react appropriately. Writing the registry entries for ably-common exposed this: each had to say "this error takes two forms, with different fixes". 104001 covered both a connect-time channel attach failure, after which nothing sends or receives, and a throw while processing one inbound message, after which the session keeps working. The second becomes 104009, so the code tells a caller whether the session is usable. 104002 covered three sites: onCancel throwing, which leaves the run NOT cancelled because the SDK never reaches the abort; onSteer throwing, where the steering message has already folded in so only the notification failed; and a cancel-dispatch failure, which is not a developer-handler fault at all. They become 104002, 104012 and 104013 respectively. 104003 wrapped run-lifecycle publishes only - the step-lifecycle publishes let the raw Ably publish error propagate, though a failed ai-step-start is the same class of failure as a failed ai-run-start. Its bracket moves to lifecycle-publish.ts and both tiers now use it, so neither can drift on the code, message shape, or cause preservation. The code is renamed to match its widened scope. The cancel-dispatch path is reachable when a run's own onError throws while reporting an onCancel failure: the second throw escapes the cancel handler and lands in the routing bracket. The new test drives it that way. check:error-codes fails until 104012 and 104013 are registered in ably-common and the submodule is bumped. 104009 is registered there for a failure nothing raises, so that entry is replaced rather than added. [AIT-1259] Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The AIT error codes are now registered, so pin ably-common at the merge of ably/ably-common#353. That brings in 104009, 104012 and 104013, which the previous commit's enum referred to before they existed, and renames 104003's identifier to run_lifecycle_event_publish_failed. protocol/errors.json no longer maps a code straight to a description string. Entries now sit under a "codes" envelope, each an object with an identifier, title and summary. validate-error-codes.ts indexed the top level, so against the bumped pin it would have found nothing and reported all 19 codes as missing - a misleading failure rather than a clean pass. It now reads codes and prints each entry's identifier, and reports an absent envelope as a stale or uninitialised submodule instead of letting it read as 19 unregistered codes. The check compares codes, not names. Generating the constants from the registry (ably/ably-common#357) is what makes the registry identifier the one spelling every SDK uses, and replaces this script. [AIT-1259] Co-Authored-By: Claude Opus 5 (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.
AIT-1259
Three of the SDK's error codes each covered several unrelated failures, so a developer could not
switchon the code and react appropriately. Writing the registry entries for ably-common#353 exposed this — each entry had to say "this error takes two forms, with different fixes", which is a smell that the code itself is wrong.The splits
connect()— nothing sends or receivesSessionSubscriptionErrorSessionMessageProcessingFailedonCancelthrew — the SDK never reaches the abort, so the run is not cancelledRunCancelHandlerFailedonSteerthrew — the steering message already folded in, so only the notification failedRunSteerHandlerFailedRunCancelRoutingFailedRunLifecycleEventPublishFailedEach now describes one failure with one recovery, and the recovery differs in each case: retry
connect()versus fix your handler; the run is dead versus the run is fine.104003 now covers steps
publishLifecycle()wrapped the run-lifecycle publishes only. The step-lifecycle publishes —runManager.startStep()andendStep()— let the raw Ably publish error propagate, though a failedai-step-startis the same class of failure as a failedai-run-start.That bracket moves to
src/core/transport/lifecycle-publish.tsand both tiers now use it, so neither can drift on the error code, message shape, or cause preservation. The code is renamed to reflect the widened scope.Naming
Members are named after their registry
identifier, which keeps arun_orsession_prefix so the AIT codes group together. These are the names that generating the constants from the registry (ably-common#357) will derive, so adopting them now avoids renaming the same members twice. The members this PR does not touch still differ from their identifiers; that migration is what settles them.ErrorCode.CancelListenerErroris removed andRunLifecycleErrorrenamed, so this is a breaking change to the exportedErrorCodeenum.ably-common
Pinned at the merge of #353.
protocol/errors.jsonno longer maps a code straight to a description string — entries sit under acodesenvelope, each an object with anidentifier,titleandsummary.validate-error-codes.tsindexed the top level, so against the bumped pin it would have found nothing and reported all 19 codes as missing. It now readscodes, and reports an absent envelope as a stale or uninitialised submodule rather than letting it read as 19 unregistered codes.Testing
check:error-codesresolves all 19 codes. New unit coverage for each newly distinguished failure: theonSteerthrow, the cancel-routing failure, and a failedai-step-start/ai-step-endpublish. The routing path is reachable when a run's ownonErrorthrows while reporting anonCancelfailure — the second throw escapes the cancel handler and lands in the routing bracket — and the test drives it that way.publishLifecycleEventgets its own test file.Full suite green: 69 files, 1677 tests.
🤖 Generated with Claude Code