DX-1211: RealtimePresence interface docstrings#2244
Draft
umair-ably wants to merge 41 commits into
Draft
Conversation
|
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 |
github-actions
Bot
temporarily deployed
to
staging/pull/2244/bundle-report
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>
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>
Static drift-guard for the ~90 inline err.hint sites DX-1209 added. For every err.hint = assignment under src/, asserts the hint string is non-empty and — if a per-code rubric exists — contains required tokens (e.g. 93001 must contain "annotation_subscribe" and "modes"; 40024 must contain the dynamic "expectedMode" identifier and "modes"; 40162 must contain "revoke" and "key"; etc). The rubric is deliberately scoped: codes shared across semantically unrelated throw sites (40000/40012/40013/40400/40500) are left out since a code-level rule would over-constrain. New entries are a 4-line block per error code. Wired into the existing Lint workflow (.github/workflows/check.yml) alongside npm run lint / format:check so drift fails PRs cheaply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prettier --write across the 9 hint-affected files (8 SDK source + the new scripts/hint-coverage.ts) and bump the minimal-Realtime bundle threshold to keep the size guard green. - Lint: prettier formatting on the trailing CLI-line additions and the multi-line hint strings. No semantic change. - Bundle: minimal-Realtime raw 107 → 116 KiB and gzip 33 → 36 KiB in scripts/moduleReport.ts. The hint extensions across the audit (capability forecasts, CLI mentions, anti-hack notes, Push admin pivot) added ~9 KiB raw / ~2 KiB gzipped to the minimal bundle. Same threshold-bump pattern fff1c1b used for the initial DX-1209 hint additions (106→107 / 32→33). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
getW3CDeviceDetails.ts: swap ErrorInfo argument order at two throw sites where (statusCode, code) had been transposed. The ErrorInfo constructor expects (message, code, statusCode); the two calls were passing (message, 400, 40000) where 400 is HTTP status and 40000 is the SDK error code. Pre-existing bug pre-DX-1209, flagged by CodeRabbit. pushactivation.ts: add missing `return;` after the GettingDeviceRegistrationFailed handleEvent call so an undefined deviceRegistration cannot fall through into GotDeviceRegistration on the next line. Without the return, the activation state machine would emit a registration event with `deviceRegistration as any === undefined`. Flagged by CodeRabbit as a critical correctness issue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hint-bearing throw sites in the SDK currently follow a 3-step pattern:
const err = new ErrorInfo('msg', 40000, 400);
err.hint = 'how to fix...';
throw err;
Add an options-object overload to ErrorInfo and PartialErrorInfo so
the same throw collapses to one call:
throw new ErrorInfo({ message, code, statusCode, hint });
The overload mirrors `fromValues` semantics: validate the shape, set
the fields, Object.assign for hint/cause/href/extras, auto-populate
href from the error code if not already set. Server-decoded errors
still flow through ErrorInfo.fromValues unchanged and continue to
carry their server-provided href and extra fields (requestId,
serverId) via the same Object.assign mechanism.
Also extend IConvertibleToErrorInfo and IConvertibleToPartialErrorInfo
to declare the hint, cause, and href fields that Object.assign has
been carrying through at runtime since the interface was introduced.
This is a type-only widening — no runtime behaviour change for any
existing caller.
Addresses AndyTWF's review-feedback question on whether fromValues
should accept hint directly so the SDK throw pattern is a single call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address AndyTWF's review-feedback question on whether the SDK should
expose hint as a constructor parameter so the throw pattern is one call
instead of three. Now that ErrorInfo (and PartialErrorInfo) accept a
values-object overload, all ~85 inline hint sites move from
const err = new ErrorInfo('msg', code, statusCode);
err.hint = 'how to fix...';
throw err;
to
throw new ErrorInfo({ message, code, statusCode, hint: '...' });
(or `const err = new ErrorInfo({...}); reject(err);` where the variable
is used by a callback). One call site that already used
ErrorInfo.fromValues({...}) for its construction (realtimepresence.ts
get-when-suspended) folds the hint into the same object literal.
Also address CodeRabbit nits in the push plugin:
- Drop the unnecessary block braces wrapping single throws/rejects in
src/common/lib/client/push.ts (2 sites) and
src/plugins/push/pushactivation.ts (3 sites) and
src/common/lib/client/auth.ts (1 site) and
src/common/lib/types/basemessage.ts (1 site).
- Extract the four-times-repeated "push.activate() registers this
process..." hint into module-level PUSH_NOT_AVAILABLE_HINT constants
in pushactivation.ts and push.ts.
No wording changes in this commit; subsequent commits will refine hint
content per Andy's substantive feedback.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address AndyTWF's substantive feedback on hint wording, factual
accuracy, and over-prescriptive server-rejection forecasts.
Wording (declarative tone, drop docs-style imperative):
- auth.ts authUrl Content-Type missing/unacceptable hints: switch
from "Have your auth endpoint return..." to "Auth endpoints may
return..."
- auth.ts unacceptable Content-Type message: "should be either" -> "Expected one of"
- "mint" word replaced with "construct" / removed in auth.ts hints
Factual corrections:
- auth.ts authUrl/token-too-large hints: acknowledge that a TokenDetails
can legitimately be large (big capability block); soften the accusation
that the endpoint is wrapping the token in extra JSON.
- auth.ts authCallback timeout hint: authUrl does not invoke a callback
and authCallback cannot return a promise; reword to differentiate the
two paths.
- auth.ts double-encoded token: trim the docs-style suffix off the
message, expand the hint with the JSON-vs-JWT diagnosis. (D1)
- connectionmanager.ts authorize 403 hint: per RSA4d the Ably server
itself can return 403 refusing to mint a TokenDetails from a
TokenRequest; broaden the hint beyond just authUrl/authCallback.
- baserealtime.ts channels.get options hint: per RTS3c, channels.get
with options is deprecated; rewrite to point at channel.setOptions()
only.
- realtimechannel.ts invalidStateError suspended branch: "suspended"
recovers automatically once the connection is re-established; only
"failed" needs an explicit attach() call.
- realtimechannel.ts/restchannel.ts publish: drop the
capability-forecast tail ("capability must include 'publish'");
there are many reasons a publish can be rejected, not just caps.
- realtimechannel.ts sync(): drop the hint entirely — sync() is an
internal SDK method, no user/LLM action is applicable.
- realtimepresence.ts enter/update/leave: drop the generic
presence-capability forecast; surface the wildcard-clientId
requirement when using enterClient/updateClient/leaveClient
on behalf of a different identity.
- realtimeannotations.ts and realtimeobject.ts (×2): "namespace must
have X enabled" reads as both an obligation and a cause; reword to
"check that the namespace has X enabled" so it's unambiguously a
hypothesis the user verifies.
- defaults.ts host-not-string hint: change example from
"main.realtime.ably.net" (a host) to "main" (an endpoint name).
- push.ts / pushactivation.ts PUSH_NOT_AVAILABLE_HINT: lead with the
supported platforms (browsers with service-worker support) before
naming the unsupported context (Node.js/server).
- utils.ts derived-channel regex: replace the placeholder "regex match
failed" message with a useful description, and swap the
transposed code/statusCode args (40010, 400 not 400, 40010) so
href auto-populates correctly under the new ErrorInfo overload.
No new throw sites added; no semantic behaviour change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace em-dashes (—) with hyphens (-) inside every err.hint string per AndyTWF's convention preference. Code-comment em-dashes elsewhere in the codebase are left alone. - Fix typo: "caes" → "case" in realtimeannotations.ts attach-state comment (CodeRabbit). No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the regex-based scanner with a ts.createSourceFile + AST walk.
Addresses AndyTWF's concern that the script is brittle, and CodeRabbit's
edge-case findings (block-comment stripping breaks inside string
literals; semicolon termination breaks inside string literals).
The new implementation walks `NewExpression` and `CallExpression` nodes
to recognise:
- new ErrorInfo({ ..., hint, code, ... })
- new PartialErrorInfo({ ... })
- <chain>.ErrorInfo({ ... }) (e.g. this.client.ErrorInfo)
- ErrorInfo.fromValues({ ... }) / PartialErrorInfo.fromValues({ ... })
For each match it extracts the `code:` numeric value and the `hint:`
string. Hint values can be plain string literals, template literals
(interpolations collapsed to their identifier text for substring
checks), string concatenations, or identifier references resolved
against same-file top-level const declarations (e.g.
PUSH_NOT_AVAILABLE_HINT).
The RUBRIC and per-code rules are unchanged from the regex version;
this commit only swaps the extractor.
After C3 (which migrated hint sites to the options-object form) the
old regex extractor was finding zero hints. The new walker finds 92
sites; all pass the rubric.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
test/unit/error-hints.test.js:
- Add `expect(err.hint).to.be.a('string')` before each substring
assertion so a missing/undefined hint fails with a clear "expected
a string" rather than a TypeError on `.contain()` (CodeRabbit nit).
- Document why `rest._FilteredSubscriptions` is the trigger for the
missing-plugin throw: no public API exposes the throw without a
Realtime connection, so we rely on the stable internal getter
(CodeRabbit fragility flag).
scripts/moduleReport.ts:
- Annotate the minimalUsefulRealtimeBundleSizeThresholdsKiB
constant: previous {raw: 107, gzip: 33}; bumped in DX-1209 to
absorb ~80 inline err.hint strings (~9 KiB raw, ~3 KiB gzip), with
recalibration guidance for future hint changes (CodeRabbit nit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sweep every (message, hint) pair in the SDK; six sites had a hint that
just rephrased the message (or vice versa). For each, keep the
"what failed" in the message and lean on the hint for the "how to fix".
utils.ts:
- derived-channel regex hint dropped the rephrased "Channel names with
derived options must look like..." (now in the message); keeps only
the docs link.
- concurrent next() hint dropped its trailing "Calling next() twice
concurrently is not supported" sentence, which mirrored the message.
restchannelmixin.ts (×3 sites):
- message trimmed to "This message lacks a serial" /
"...and cannot be updated"; the "Make sure you have enabled X in
channel settings on your dashboard" guidance already lives in the
hint and is dropped from the message.
realtimechannel.ts:
- message-lacks-serial site: same trim as restchannelmixin.
- detach-from-failed hint dropped the "A failed channel cannot be
detached" sentence (a rephrasing of "Unable to detach; channel
state = failed").
- attach/detach-timeout hints dropped the "server did not acknowledge
within realtimeRequestTimeout" sentence (a rephrasing of "Channel
attach/detach timed out").
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The decoding loop in basemessage.ts catches per-encoding errors and rewraps them with an outer ErrorInfo so the caller gets one error per message rather than one per encoding stage. The wrapper used the positional ErrorInfo constructor, which has no hint parameter, so the three hint-bearing throws in this loop - Vcdiff plugin missing (40019), no typed-array support (40020), Vcdiff decode failure (40018) - had their hints silently swallowed before reaching the caller. Pre-DX-1209 this was harmless because no inner hints existed; flagged by CodeRabbit review. Switch the wrapper to the options-object constructor and forward err.hint. Cause is intentionally not forwarded: three of the inner throws are plain Error (cipher and 'Unknown encoding' branches) which do not fit cause: ErrorInfo | PartialErrorInfo without an instanceof branch, and CodeRabbit flagged cause as optional. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The _getDeviceIdentityToken() throw site in pushchannel.ts is reached from both subscribeDevice() and unsubscribeDevice() (via the shared _getPushAuthHeaders() helper), but its message and hint only mentioned subscribing. CodeRabbit flagged that this misdirects users hitting the error from an unsubscribe call. Rewrite both message and hint to be action-neutral. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pinning exact hint substrings (and the static AST coverage check that guarded token presence) treats hints like a tested API surface. We don't test exact error-message wording elsewhere, and the same reasoning applies to hints — they're guidance, not a contract. Remove the apparatus rather than maintain drift assertions for it. Removes: - scripts/hint-coverage.ts (+ npm run hintcoverage, CI step in check.yml) - test/unit/error-hints.test.js Guidance on writing/using hints will live in a best-practices doc instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per PR review, message and hint must not overlap: message states only "what" went wrong; hint states only "how" to fix it. A workflow assessed all 93 message/hint pairs and tightened the 19 remaining offenders: - Stripped restated-"what" preambles from hints (realtimepresence, connectionmanager, basemessage, auth, push, pushchannel, rest, paginatedresource, defaults, getW3CDeviceDetails). - Reduced messages that carried remediation already present in the hint (wildcard-clientId in auth/baseclient; setOptions guidance in baserealtime). No actionable guidance lost - removed text was duplicated in its counterpart. tsc --noEmit clean for all edited files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
umair-ably
force-pushed
the
DX-1211/realtimechannel-docstrings
branch
from
bec860c to
ae2344a
Compare
umair-ably
force-pushed
the
DX-1211/realtimepresence-docstrings
branch
from
0c60917 to
1da8dbb
Compare
github-actions
Bot
temporarily deployed
to
staging/pull/2244/bundle-report
Address lmars' 2026-06-29 review comments plus a PR-wide accuracy sweep,
each verified against the SDK code paths, state machines, and ably.com docs:
- auth.ts: clarify 40160 client-side vs server-side auth guidance (docs-aligned);
split the 40102 message vs hint; reframe the 40170 "too large" family
(envelope + capability/wildcards) consistently; tighten the callback
return-shape and createTokenRequest hints; clarify clientId "*" (a fixed
identity cannot be "*"; a wildcard identity comes from a wildcard token)
- baseclient.ts: drop the token/tokenDetails suggestion on an invalid key;
remove the irrelevant wildcard tangent on a non-string clientId; clarify
clientId "*"
- paginatedresource.ts: de-jargon the first()/current() hints
- realtimeannotations.ts, realtimeobject.ts: recommend channel.setOptions to
change modes (channels.get(name, { modes }) throws on an attached channel)
- realtimechannel.ts: align the detach/attach-timeout recovery hints with the
channel state machine (attach() recovers from failed; suspended auto-retries)
- realtimepresence.ts: correct leave() guidance per state (presence members
are cleared on failed/detached, so there is nothing to leave)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ll-PR docs review Verified findings from the 2026-07-02 docstrings-pr-review run (each traced against code paths, state machines, CLI output, and a live sandbox trace): - utils.ts: derived-channel 40010 message was false (branch fires only on empty/unparseable names, and the hint's own example triggered a different error); createMissingPluginError now names the real entry point per plugin (Push -> ably/push, LiveObjects -> ably/liveobjects, others -> ably/modular); v1-callback message/hint reworded (tests re-pinned) - defaults.ts: drop invented v1/v2 version framing (endpoint arrived in 2.10.0 per CHANGELOG; environment/restHost/realtimeHost are deprecated v2 options); fallbackHosts hints are now single prescriptions - realtimeobject.ts (LiveObjects): 40024 hints no longer claim the attach is rejected on a capability shortfall (live-traced: the server resolves the attach and silently drops the mode) and no longer cite a nonexistent "LiveObjects namespace" dashboard setting; recipe aligned with the sibling missing-mode hints - realtimechannel.ts: invalid-mode hint no longer claims the attach is rejected (silently-dropped story, matching channel.modes); 40003 sendUpdate hint unified to the canonical missing-serial construction; untilAttach message now carries the observed state like the presence twin - realtimepresence.ts: enter/update 40012 hints share one skeleton; leave 40012 no longer prescribes a no-op set-clientId-and-retry errand - connectionmanager.ts: 403 auth hint no longer narrates a server-refusal branch that never fires on that path; ping hint covers the closed/failed/initialized states that never auto-connect - basemessage.ts: vcdiff 40018 recovery hint matches the traced behaviour (re-attach from last processed message; delta stays enabled) - push cluster: platform-unavailable messages now diagnose (browser/service worker requirement), hints are pure prescriptions; subscribeClient/ unsubscribeClient hints state the real clientId sources per client type; getW3CDeviceDetails branches denied vs dismissed permission - rest.ts: revokeTokens hint names the real mechanism (revocable tokens enabled on the issuing key), not a nonexistent "revoke capability" - restchannel/restchannelmixin/restannotations/paginatedresource/baserealtime: canonical missing-serial constructions, single-prescription hints, message mechanics; 40009 max-size message parenthetical unified across the three publish sites - auth.ts/baseclient.ts: clientId type-check hints byte-identical; residual diagnosis clauses moved from hints into messages Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
umair-ably
force-pushed
the
DX-1211/realtimepresence-docstrings
branch
from
1da8dbb to
31891bf
Compare
umair-ably
force-pushed
the
DX-1211/realtimechannel-docstrings
branch
from
ae2344a to
1d45ea3
Compare
github-actions
Bot
temporarily deployed
to
staging/pull/2244/bundle-report
Fact-bearing parentheticals fold into the clause or become their own
sentence per the hint style bar. The LiveObjects mode hints keep the
guard that channels.get(name, { modes }) on an existing channel throws,
steering the caller between the two suggested remedies.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Declaration only — no runtime use yet. Documented silent-failure paths will read this option in subsequent commits to gate a hint-carrying throw; the default stays `false` in v2.x. Per DXRFC-022 work item B5 the default flips to `true` in v3 with no per-call opt-out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The guard at line 73 parenthesised `(state === 'attached' && _mode & flag) === 0` so the `=== 0` compared against `(boolean && number)`. It happened to behave correctly — `false === 0` is `false` so the throw skipped when not attached, and the bitwise result === 0 was correct when attached — but the comparison was structural luck rather than the obvious reading of the predicate. Re-parenthesise to `state === 'attached' && (_mode & flag) === 0`. Also emit an always-on warning log adjacent to the throw so the diagnostic fires in the SDK output even when the caller swallows the throw. No silentFailureLogSuffix here because this throw is unconditional (pre-DXRFC-022) and not strictMode-gated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A short suffix appended to silent-failure warning logs emitted when clientOptions.strictMode is off, so the reader knows the same path will throw in a future major version. Co-locating on Logger keeps the import surface tight; callers do `Logger.silentFailureLogSuffix()` next to `Logger.logActionNoStrip(...)`. Log-only by design — the suffix is not put into ErrorInfo.hint, because the hint is also shown when the throw fires (strictMode on), where the suffix would be misleading. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a channel was attached without the presence_subscribe mode, the
server never delivers presence members, so presence.get() resolves to
[] regardless of who is actually present. Today there is no signal
distinguishing "no one is present" from "this client cannot see anyone".
This commit detects the case after ensureAttached() populates the
server-granted mode set, then:
- emits an always-on warning log carrying the hint + a suffix telling
the reader that strictMode will throw in a future major version.
- throws ErrorInfo with err.hint when clientOptions.strictMode === true.
Code 93002 sits next to 93001 (annotation_subscribe missing) in the
SDK-internal precondition class. It would also be defensible to use
40160 (server-side capability denied), but the hint-coverage rubric
already pins 40160 to the "no auth options" hint shape, so a second
40160 throw site would either weaken that pin or need a rubric refactor.
Suspended-state and {waitForSync: false} paths return earlier and are
unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a channel was attached without the subscribe mode, the server never delivers messages to the listener, so channel.subscribe() appears to succeed but no callback ever fires. This commit closes the gap symmetrically to presence.get() in the previous commit: - After the implicit attach completes (RTL7g), if subscribe mode was not granted by the server, emit an always-on warning log carrying the hint + the future-throw suffix. - One-shot per channel: the warning fires once per attach cycle to keep noise down on long-lived listeners. Reset _silentSubscribeWarned on the ATTACHED message so a channels.release() + re-attach with corrected modes restores signalling. - Throw ErrorInfo (code 93003) when clientOptions.strictMode === true. attachOnSubscribe: false is out of scope — the check requires an attach to have populated _mode. Document this caveat separately if needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Terser, technical error messages for 93002/93003 - Hints now offer setOptions() or omitting modes + capability check; drop confusing channel.modes note - Keep listener registered on strict-mode throw (matches existing subscribe semantics) - Remove ticket IDs from test names Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…grade The 93001 hint lumped two distinct failure modes under "if the subsequent attach is rejected": a missing namespace rule (Mutable Messages) does reject the attach, but a missing annotation-subscribe capability does NOT - the server silently drops the mode and this same error re-fires. Confirmed by live sandbox traces and the canonical error registry. Reword to key off the symptom the caller observes: attach rejected => enable the namespace rule; attach succeeds but annotations still undelivered => grant the capability. Addresses review feedback that the hint was presumptuous and over-enumerated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… off colliding 93002 Two fixes to the presence.get()/channel.subscribe() missing-mode errors: - Hint correctness (review feedback): the presence hint named a "presence-subscribe" capability that does not exist. Presence delivery is governed by the "subscribe" capability, matching the subscribe-mode hint. - Error-code collision: 93002 is the canonical server code for "namespace needs Mutable Messages" (ably-common errors.json, faqs.ably.com/error-code-93002). presence.get() reused it client-side. Move presence to 91008 (presence block, next to 91005) and channel.subscribe() to 90009 (channel block); 93xxx is the annotations/mutable-messages block. Both codes are new on this unreleased branch, so the renumber is non-breaking. Reserved in ably/ably-common#345. Update the two tests asserting them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…align the silent-failure log suffix From the 2026-07-02 docstrings-pr-review run: - ClientOptions.strictMode: both strictMode-gated paths are async, so the caller-observable failure is a rejected promise, not a throw. Also drop the vague "documented silent-failure paths" (D10), the version label (C9), the presence.get() worked example (B6), the parenthesised default (A5), and the em-dash opt-out clause (B5/A2). Default stated once, late, as its own sentence. - Logger.silentFailureLogSuffix: fold the JSDoc into two one-line paragraphs, fix ClientOptions casing, drop the "(human or LLM)" aside, and align the suffix string with the strictMode docstring framing (default will change in a future major version; enabling it now makes the call reject). Also re-verified by live sandbox trace: a namespace without the annotations rule rejects the attach with 93002, while a capability shortfall resolves the attach and silently drops the mode. The 93001 hint's reject-vs-downgrade split is therefore correct as shipped and is deliberately left unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-get mode hints Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…capability branch
Drops the attach-rejection narration, which surfaces as its own error,
and promotes the parentheticals to sentences. Keeps the guard that
channels.get(name, { modes }) on an existing channel throws, steering
the caller between the two suggested remedies.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ts, failure modes) Apply docstringRules.md to the public Auth interface in ably.d.ts so silent and architectural call-site prerequisites are discoverable at the call site: - revokeTokens: basic-auth (API key, not token) requirement; non-code "revocable tokens enabled on the key" prerequisite, with a feature-page @see. - requestToken: token-issuing prerequisite (authCallback/authUrl/key); callback contract detail (content-types, size flags) offloaded to the @see. - createTokenRequest: a local API key must be available to sign the request. - authorize: re-authenticates the live connection (resolves once the token takes effect on a connected connection), token-issuing prerequisite, and the RSA10a key-immutability rule (authorize() cannot change the API key). - clientId: adds the canonical @see. Folded prose, no numeric error codes; every behavioural method carries one simple @example and every member an @see to the canonical JS API reference. TypeDoc (treatWarningsAsErrors), eslint, and prettier are clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the docstrings-standards.md review pass to the Auth interface, incorporating m-hulbert's PR #2242 suggestions: - Split walls of text into blank-line paragraphs, shorten sentences, and remove semicolons from descriptions (A1/A2). - authorize: five paragraphs with active reject voice and an ErrorInfo on each failure mode. - createTokenRequest: promote the defaults parenthetical to a sentence and correct the reviewer's "TokenRequest" arg slip (the method takes no TokenRequest parameter). - requestToken/createTokenRequest: one verbatim stored-defaults phrasing, and authorize() with parentheses (C5/F2). - revokeTokens: anchor the basic-auth prerequisite to the calling client to remove the ambiguity m-hulbert flagged (D5). - clientId: drop the inline identified-clients link now that the canonical @see is present (E1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the 2026-07-02 docstrings-pr-review run, re-verified against the post-review-feedback text: - clientId: rewrite the ClientOptions-copied description around the identity actually in effect on this client, how it resolves (options or token), and the 40102 conflict, anchored to the traced auth.ts behaviour - authorize: the prerequisite was anchored to the wrong subject and claimed the call rejects when ClientOptions lacks a token source; authorize() saves passed authOptions before validating, so the requirement is on the resolved AuthOptions (including a directly supplied token) - requestToken: drop the REST-endpoint mechanism opener; split purpose, stored defaults, and prerequisite into their own paragraphs - createTokenRequest: drop the stored-defaults sentence that duplicated the paragraph below it - stored-defaults concept now phrased identically across authorize, createTokenRequest, and requestToken Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… at the prerequisite Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… side-effects, failure modes) Surface call-site prerequisites and failure modes across the RealtimeChannel public surface per docstringRules.md (terse folded prose + @see companion): - subscribe(): silent missing-`subscribe`-mode trap + strictMode aside (DX-1211 core) - params/errorReason: guard against undefined/null despite the declared type - history(): persistence channel-rule prerequisite; fix "presence members" @param bug - message ops (get/update/delete/append/getMessageVersions): async-reject framing and message-interactions channel-rule prerequisite - push: universal missing-plugin throw kept; presence/annotations left as-is (bundled by the default build) TypeDoc (treatWarningsAsErrors), prettier, and eslint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply reviewer suggestions and corrections from m-hulbert and AndyTWF:
- errorReason, params, push, history: apply the reviewers' suggested
wording; drop the inline docs link on params.
- push: state that neither the default nor modular build bundles the
Push plugin and to import it from `ably/push`; fix naming
("push notification", backticked `Push`).
- Remove deprecated/incorrect terms across the message-mutation methods:
"message interactions" and "channel rule" -> "a rule"; "durable
storage" -> "storage". State the populated-serial requirement plainly.
- attach/detach: drop internal EventEmitter references, describe the
emitted state change as prose; add implicit-attach triggers (presence
update()/get(), LiveObjects get()) and the detaching state; note that
detaching stops server delivery while local listeners remain; link
release()/get().
- subscribe: collapse the mode-gate narration to a one-line headline.
- publish: drop the sibling-contrast sentence; unify the verb mood.
- setOptions: remove the "live re-attach" coinage.
- unsubscribe: state the no-detach consequence (server keeps sending,
billed) consistently across all overloads.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Channel docstrings
From the 2026-07-02 docstrings-pr-review run (per-member review, adversarial
verify, reviewer-lens, and cross-unit consistency barrier), each fix traced
against the implementation:
- unsubscribe (all 7 overloads): drop the banned billing consequence for the
capability-qualified streaming form; drop restating participials; state
that the filter overload matches by object identity and that an unknown
filter removes nothing and no error is raised; drop the signature-obvious
example on the event overload
- subscribe (all 4 overloads): name the silent failure ("and no error is
raised"); document the second null resolution (attachOnSubscribe: false, no
attach attempted); drop the restated listener sentence on two overloads
- attach: receiver-qualify and link every implicit-attach trigger (bare
"subscribe()" was ambiguous); replace undefined "connection is unusable"
with the traced connection states (suspended, closing, closed, failed);
name the state-change listener surface (on()/once()), also on detach
- detach: cover presence/annotation/object traffic, not just messages;
sanctioned lifecycle-state constructions
- setOptions: only changed modes/params wait for the next attach; other
options, such as a cipher, take effect immediately (traced: channelOptions
and decoding context update synchronously)
- params: drop the causeless may-differ clause; split the undefined guard
into its own paragraph. modes: split the 32-word sentence, state that no
modes granted yields undefined, not an empty array
- push: drop bundling/import mechanism (belongs at the plugin docs); split
purpose from prerequisite; end the failure sentence at the throw
- whenState: a state never reached means the promise never settles and no
error is raised
- deleteMessage: restore the fact dropped from main that a delete keeps the
previous data unless explicitly overwritten; appendMessage: example now
shows the spread shape (passing the received message verbatim would re-send
its existing data)
- mutable-message members: link the rule prerequisite to
https://ably.com/docs/channels#rules (anchor verified live); REST Channel
siblings get the same serial-prerequisite and rule-gate paragraphs,
canonical patch-semantics sentence, and mirrored examples (they share the
RestChannelMixin code paths byte-for-byte; @see mirroring deferred, no REST
reference page exists in ably/docs #3400)
- history: persistence caveats split into their own paragraph
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… apply the suggested push docstring Moves the storage concept inline onto the persistence-rule mention per the single-link convention, and restores the Push plugin bundling sentence via the reviewer's suggested rewrite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, side-effects, failure modes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…trings to sentences; single @see on history() Aligns presence get/subscribe mode wording with the reviewed RealtimeChannel form, and moves the storage concept inline onto the persistence-rule prerequisite per the single-link convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
umair-ably
force-pushed
the
DX-1211/realtimechannel-docstrings
branch
from
d71092a to
c064311
Compare
umair-ably
force-pushed
the
DX-1211/realtimepresence-docstrings
branch
from
e2de3ee to
9e59a94
Compare
github-actions
Bot
temporarily deployed
to
staging/pull/2244/bundle-report
umair-ably
force-pushed
the
DX-1211/realtimechannel-docstrings
branch
from
c064311 to
d5cf195
Compare
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
1 participant
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.
Stacked on #2243 (RealtimeChannel docstrings, base
DX-1211/realtimechannel-docstrings); review/merge that first. Rewrites the JSDoc on theRealtimePresencepublic surface inably.d.tsperdocstringRules.md: terse folded prose that surfaces call-site prerequisites, side-effects, and failure modes (the DX-1211 silent-failure class), with an@seecompanion link to the canonical reference.Scope
ably.d.tsonly (+83/−19). 11 members rewritten (counting overload groups once).Highlights
subscribe()(DX-1211 core): documents the silent missing-presence_subscribe-mode trap — the channel attaches but the server never delivers presence events, so the listener silently never fires; also notesattachOnSubscribe: falseskips the implicit attach. On both active overloads.get(): same mode prerequisite, but the failure shape differs — resolves with an empty array rather than rejecting (or rejects with a hintedErrorInfounderstrictMode).syncComplete: precise semantics — flips back tofalseon a new sync, istrueafter the local set is cleared, so it only means "no sync in progress", not "attached"; points toget()as the way to wait for a sync.enter()/update()/leave(): identified-client prerequisite (rejects on missing/wildcardclientId), with pointers to the*Client()variants;enter()documents automatic re-enter on re-attach and that a re-enter failure surfaces as a channelupdateevent, not a rejection.leave(): unlikeenter(), does not implicitly attach — proceeds only whileattached/attachingand rejects otherwise.enterClient()/updateClient()/leaveClient(): wildcard-clientIdkey/token prerequisite is server-enforced, so violations reject with a server-returnedErrorInfo.unsubscribe(): all six overloads clarify the removal is local-only — no detach, no presence leave.history(): non-code persistence channel-rule prerequisite (72h vs the 2-minute default retention).Validation
npm run docs(TypeDoctreatWarningsAsErrors),prettier --check, andeslintall pass. A clean TypeDoc build confirms every{@link}resolves and nothing became undocumented.Reviewer notes
@seeanchors follow the kebab-case convention (matching the Auth and RealtimeChannel PRs) but are unverified — ably/docs #3400 isn't live yet and@seeis not TypeDoc-validated.attachingcan resolve silently without ever reaching the server in some failure paths); it is deliberately not documented here pending a decision on whether to fix the code instead.🤖 Generated with Claude Code