Skip to content

[FSQL] Prevent client socket errors from escalating to pglite server crashes#10845

Merged
mtr002 merged 1 commit into
mainfrom
mtr-em-fix
Jul 24, 2026
Merged

[FSQL] Prevent client socket errors from escalating to pglite server crashes#10845
mtr002 merged 1 commit into
mainfrom
mtr-em-fix

Conversation

@mtr002

@mtr002 mtr002 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes Issue#10821

Cancelling an in-flight Data Connect request makes the Go backend Postgres driver send a CancelRequest frame on a new TCP connection. pg-gateway throws Unexpected initial message, which pgliteServer.ts currently re-emits as a server error—causing dataconnectEmulator.ts to call cleanShutdown() and stop all emulators.

This PR decouples client socket errors from server-level errors in pgliteServer.ts. Per-socket errors are now logged and destroyed so single connection drops don't crash the emulator stack. Real server errors (EADDRINUSE) are still handled normally.

Scenarios Tested

  • Sent CancelRequest packet to port while emulators were running; verified stack stayed alive and served subsequent GraphQL queries.
  • Verified reproduction crash on npx firebase-tools@latest vs survival on local fix.
  • Ran full emulator test suite and npm run lint:changed-files.

Sample Commands

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the error handling for Postgres client sockets in pgliteServer.ts. Instead of escalating client socket errors to server-level errors, they are now logged as debug messages and the socket is destroyed. I have no feedback to provide.

// to server-level errors.
socket.on("error", (err) => {
server.emit("error", err);
logger.debug("Postgres client socket error (expected during request cancellation):", err);

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.

I think this is fine but the logs can be a bit misleading if it happens outside of cancellation. Maybe add as a newline suffix e.g. "If this happened during request cancellation, it's expected and harmless"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. Done

@yuchenshi

Copy link
Copy Markdown
Member

Please add an CHANGELOG entry

@mtr002
mtr002 enabled auto-merge (squash) July 24, 2026 21:43
@mtr002
mtr002 merged commit 8fe0114 into main Jul 24, 2026
52 of 53 checks passed
@mtr002
mtr002 deleted the mtr-em-fix branch July 24, 2026 21:52
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