fix(ts-sdk): prevent uncaught exceptions from breaking message pipeline#4756
fix(ts-sdk): prevent uncaught exceptions from breaking message pipeline#4756DexterKoelson wants to merge 1 commit intoclockworklabs:masterfrom
Conversation
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.
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?
I think ideally we'd like to wrap just the user callbacks in the |
Fixes #4755
After #4744 replaced promise chained message processing with a synchronous drain loop, we started seeing false
Received SubscriptionError for unknown querySetIdconsole errors.The old promise chain approach had a subtle bug: if any
#processMessagecall threw an exception (e.g. from a user callback inonInsert/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 viatry/finally, but defers remaining messages until the nextonmessageevent. This means messages that were previously swallowed, including benignSubscriptionErrorresponses for already-cleaned-up subscriptions, are now actually processed and logged as errors.This PR makes two changes:
Isolate message failures in the drain loop: wrap
#processMessageintry/catchso 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.Downgrade "unknown querySetId" logs from
errortowarn: these are benign race conditions (e.g. server responding to a failed unsubscribe after the subscription was already cleaned up by a priorSubscriptionErrororUnsubscribeApplied) 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