Skip to content

Make treatment of session disconnect and transport closing similar#323

Merged
masad-frost merged 2 commits intomainfrom
fm-similar
Jun 12, 2025
Merged

Make treatment of session disconnect and transport closing similar#323
masad-frost merged 2 commits intomainfrom
fm-similar

Conversation

@masad-frost
Copy link
Copy Markdown
Member

Why

After the transport closes, calling a procedure from the client leads to throwing an error telling you transport is closed. This happens because we end up creating a new session and then failing at getting the bound send

What changed

We no longer expose the status of the transport to the caller, we pretend that your session suddenly disconnected, unifying the interface when reading after session closed and calling after transport closed.

Versioning

  • Breaking protocol change
  • Breaking ts/js API change....kinda

@masad-frost masad-frost requested a review from a team as a code owner June 11, 2025 21:59
@masad-frost masad-frost requested review from daweifeng-replit and removed request for a team June 11, 2025 21:59
{
path: '/mustSendThings',
message: 'Expected required property',
message: 'Required property',
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unrelated change that came with the latest typebox upgrade.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, my nodE_modules are outdated

Copy link
Copy Markdown
Contributor

@daweifeng-replit daweifeng-replit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@masad-frost masad-frost merged commit 48d9317 into main Jun 12, 2025
9 checks passed
@masad-frost masad-frost deleted the fm-similar branch June 12, 2025 17:43

const writable = new WritableImpl<Static<PayloadType>>({
writeCb: () => {
// noop
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably error as the writable we close on UNEXPECTED_DISCONNECT in the normal case iirc so this should match?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants