fix(js/net)!: separate session and stream errors - #2794
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b416929a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Which registry {@link code} belongs to. */ | ||
| readonly source: RemoteSource; | ||
|
|
||
| constructor(code: number, options: { source: RemoteSource; cause?: unknown; reason?: string }) { |
There was a problem hiding this comment.
This makes the exported RemoteError constructor's previously optional options argument required, but the reviewed commit is based directly on main (76a1aedd). If landed there, existing @moq/net consumers that call new RemoteError(code) receive a breaking change outside the designated breaking-release line. Rebase or retarget this change onto dev, which the repository explicitly reserves for signature changes to exported library APIs. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L184-L186
Useful? React with 👍 / 👎.
SessionCode.Unauthorized and StreamCode.DeliveryTimeout are both 2 in disjoint registries. A single RemoteError carrying an untyped number let reconnect logic and reset forwarding interpret a code against the wrong registry. Replace it with SessionError and StreamError carrying branded SessionCode and StreamCode values. Race SETUP negotiation against the session close so an authorization rejection during the handshake reaches the terminal retry classifier, while stream resets remain retryable. BREAKING CHANGE: RemoteError is replaced by SessionError and StreamError, and their code fields use distinct branded registry types.
7b41692 to
a576462
Compare
kixelated
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5764620d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** A code from the stream reset registry. */ | ||
| export type StreamCode = number & { readonly [STREAM_CODE]: true }; |
There was a problem hiding this comment.
When a publisher uses an application-defined reset code from the documented 64+ range, the new opaque brand makes group.close(new StreamError(64)) fail type-checking, while StreamCode exposes only the fixed protocol constants and no public factory. Because publisher errors eventually reach stream.reset and only a StreamError preserves its code, applications can no longer send their own reset codes without an unsafe assertion; expose a validated constructor or factory for application codes. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L133-L135
Useful? React with 👍 / 👎.
| * | ||
| * @public | ||
| */ | ||
| declare const SESSION_CODE: unique symbol; |
There was a problem hiding this comment.
The detailed registry comment now attaches to the private SESSION_CODE declaration rather than the exported SessionCode constant, leaving the public value undocumented in generated .d.ts and JSR API docs; the same displacement occurs for StreamCode. Move each registry comment directly above its exported constant and give the private brand declaration its own internal comment. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L92-L95
Useful? React with 👍 / 👎.
1 participant
Summary
SessionCode.UnauthorizedandStreamCode.DeliveryTimeoutare both0x2in disjoint registries. The previousRemoteErrorcarried an untyped number, so reconnect logic could interpret a SETUP stream reset as a terminal authorization rejection. Reset forwarding could also send a session code back as a stream code.Replace the shared error with registry-specific types:
SessionError.code: SessionCodeStreamError.code: StreamCodeSessionCodeandStreamCodeare branded numeric types. The transport decode boundaries apply the correct brand, including to unknown or application-defined codes, while TypeScript rejects mixing the registries.The SETUP negotiation now races
session.closed. An unauthorized close during the handshake becomes aSessionErrorand stops reconnecting, rather than being hidden behind a failed or stalled setup stream. Stream reset code 2 remains a retryableStreamError.Reset forwarding now preserves only
StreamErrorcodes. Session and local errors take the plain-error path instead of being mistranslated onto a stream.Public API changes
Breaking, targeting
dev:RemoteErrorandRemoteSource.SessionErrorandStreamError.SessionCodeandStreamCodeso they cannot be mixed or replaced with raw numbers.@moq/netremains on the existing unreleased0.3.0line, so no additional version bump is needed.Test plan
nix develop --command just fixnix develop --command just checknix develop --command just testCross-Package Sync: updated the
@moq/netdocumentation and subscribe example. No wire format changed.(Written by GPT-5.6 Sol)