Skip to content

fix(ts-sdk): prevent uncaught exceptions from breaking message pipeline#4756

Open
DexterKoelson wants to merge 1 commit intoclockworklabs:masterfrom
DexterKoelson:fix/ts-sdk-unknown-queryset-error
Open

fix(ts-sdk): prevent uncaught exceptions from breaking message pipeline#4756
DexterKoelson wants to merge 1 commit intoclockworklabs:masterfrom
DexterKoelson:fix/ts-sdk-unknown-queryset-error

Conversation

@DexterKoelson
Copy link
Copy Markdown
Contributor

Fixes #4755

After #4744 replaced promise chained message processing with a synchronous drain loop, we started seeing false Received SubscriptionError for unknown querySetId console errors.

The old promise chain approach had a subtle bug: if any #processMessage call threw an exception (e.g. from a user callback in onInsert/onApplied), the promise chain broke and all subsequent messages were silently dropped forever and into the void. The new drain loop correctly recovers from exceptions via try/finally, but defers remaining messages until the next onmessage event. This means messages that were previously swallowed, including benign SubscriptionError responses for already-cleaned-up subscriptions, are now actually processed and logged as errors.

This PR makes two changes:

  1. Isolate message failures in the drain loop: wrap #processMessage in try/catch so a failure in one message (e.g. a user callback throwing) doesn't interrupt processing of subsequent messages. The error is logged and the loop continues.

  2. Downgrade "unknown querySetId" logs from error to warn: these are benign race conditions (e.g. server responding to a failed unsubscribe after the subscription was already cleaned up by a prior SubscriptionError or UnsubscribeApplied) and don't indicate data corruption.

API and ABI breaking changes

None

Expected complexity level and risk

1 -- minimal, localized change to error handling and log levels.

Testing

  • Existing test suite passes (170/170).
  • My local use continues to function without the error

The synchronous drain loop introduced in 2a8718b correctly recovers
from exceptions, unlike the old promise chain which silently dropped
all subsequent messages after a throw. However, the recovery still
deferred remaining messages until the next onmessage event.

Wrap #processMessage in try/catch so each message is isolated — if one
fails (e.g. from a user callback), the rest continue immediately.

Also downgrade "unknown querySetId" logs from error to warn. These are
benign race conditions (e.g. server responding to a failed unsubscribe
after the subscription was already cleaned up) and not data corruption.
@joshua-spacetime joshua-spacetime requested a review from gefjon April 7, 2026 15:35
@gefjon
Copy link
Copy Markdown
Contributor

gefjon commented Apr 7, 2026

Downgrade "unknown querySetId" logs from error to warn: these are benign race conditions (e.g. server responding to a failed unsubscribe after the subscription was already cleaned up by a prior SubscriptionError or UnsubscribeApplied) and don't indicate data corruption.

I don't know that this is benign, actually. Our message format should be totally ordered and consistent, and the server should never be sending more than one subscription-ending message for the same subscription. This sounds like a bug, either in the TypeScript SDK or in the SpacetimeDB host. I'd much prefer to fix that bug and leave the error as-is. Do you have a consistent reproducer so we can debug?

wrap #processMessage in try/catch so a failure in one message (e.g. a user callback throwing) doesn't interrupt processing of subsequent messages. The error is logged and the loop continues.

I think ideally we'd like to wrap just the user callbacks in the try/catch, and preserve exceptions from as much of the SDK code as possible. If we partially process a message, then bail due to an exception as a result of a bug in the SDK, that potentially leaves the client cache in an invalid state, and we'd rather not continue processing additional messages and risk showing incorrect data to the user.

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.

TS SDK logs false "Received SubscriptionError for unknown querySetId" errors

2 participants