fix: api event-dedup ordering, news stream errors, redis parse guard (#532, #546, part of #557)#351
Open
Flegma wants to merge 2 commits into
Open
fix: api event-dedup ordering, news stream errors, redis parse guard (#532, #546, part of #557)#351Flegma wants to merge 2 commits into
Flegma wants to merge 2 commits into
Conversation
…e guard - MatchEventsGateway: write the dedup cache entry only after the processor succeeds, and wrap processing in try/catch that logs and rethrows. A transient failure no longer both throws and leaves a 'processed' marker that suppresses the game server's redelivery for the 10s TTL (#532). - NewsController.serveImage: handle 'error' on the S3 stream and destroy it on response close, so a stream error can't crash the process and a client disconnect can't leak the upstream fd (#546). - SocketsService: guard JSON.parse of pub/sub messages so a malformed payload logs and returns instead of throwing an uncaught exception in the message listener (#557, redis-handler half). Adds match-events.gateway.spec.ts covering the dedup ordering.
…ollow-up) JSON.parse succeeds for a valid non-object payload (e.g. literal null, which typeof-reports as object), so the destructure sat outside the try and would throw an uncaught TypeError. Validate parsed is a non-null object first.
Contributor
Author
|
Independent code review run on the diff. It caught that the redis parse guard wrapped only |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three self-contained reliability fixes from the 2026-07 audit. Closes #532, #546; partially addresses #557.
Changes
Match-event dedup ordering (#532)
MatchEventsGateway.handleMatchEventwrote the dedup cache entry before running the processor, so a transient processor failure (for example a Hasura hiccup) left a "processed" marker that suppressed the game server's redelivery for the 10s TTL, silently dropping the event. The entry is now written only afterprocess()resolves, and processing is wrapped in try/catch that logs with the stack and rethrows (so redelivery reprocesses). Affects every event type.News image stream (#546)
NewsController.serveImagepiped the S3 stream to the response with no handlers. An upstream stream error threw an unhandled'error'event (crashing the pod), and a client disconnecting mid-download left the upstream stream open (fd leak). Added anerrorhandler (log + 500/destroy) and a responseclosehandler that destroys the stream.Redis pub/sub parse guard (#557, one half)
SocketsServicecalledJSON.parseon everybroadcast-message/send-message-to-steam-idpayload inside thesub.on("message")listener with no guard, so a malformed message was an uncaught exception in an event handler (pod crash). Now wrapped in try/catch that logs and returns.Testing
tsc --noEmit: no new errors (the 19 pre-existing errors are all insrc/matchmaking/matchmake.service.spec.ts, tracked as #534).match-events.gateway.spec.ts(3 tests): dedup entry written only after success, not written on failure (so redelivery retries), and dedup short-circuit skips processing. Passing.Deferred from this PR (need a decision, documented on the issues)
x-origin-auth === nodeIdis tautological becausenodeIdis a public path param. Fixing it needs a real per-node shared secret (same class as the connector auth gap and #413), so it is not a self-contained change.