fix(networking): FIFO-queue api_client responses so concurrent requests don't collide#10
Merged
V-rtualized merged 2 commits intoJul 21, 2026
Conversation
…ts don't collide The api_client kept a single pending_callback and a single, self-clearing mqtt.on_http_response slot. Two requests in flight at once overwrote each other: the second request replaced the first's handler, so when the first response arrived it ran the wrong parser (surfacing as "Failed to parse server response") and the second response was dropped entirely. This broke any flow that issues two requests close together -- notably the queue-guard "Leave Queue & Continue" firing leave_matchmaking_queue and then join_lobby in the same tick, which made the join fail while a direct join (single request) worked. The worker thread (mqtt_thread.lua) drains tx_channel in order and runs each HTTP request synchronously, so http_response/http_error events come back in send order. Replace the single slot with a FIFO queue of handlers plus a persistent router that pops the front on each event -- matching every response to its request. Convert the remaining hand-rolled single-slot handlers (account link/unlink, lobby set_metadata, enable_chat, send_chat_message) onto the same queue so nothing bypasses it. Adds dev/test_api_client_fifo.lua: two overlapping requests each receive their own response (success and error routing), plus a control transcribing the old single-slot design to prove it misroutes/drops (reproduces the bug).
This was referenced Jul 13, 2026
Merged
…tripwire Add a warn when a response/error arrives with no pending request -- a canary for response-queue desync. It is impossible today (the worker runs requests serially so responses are FIFO; MPAPI.reconnect rebuilds a fresh client rather than swapping a live transport), but it warns loudly instead of silently misrouting if a future change breaks either invariant. Document the three invariants FIFO matching depends on. Test covers the tripwire firing.
This was referenced Jul 14, 2026
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.
Two overlapping HTTP requests through
api_clientcollide: one callback never fires and the other receives the wrong response body. This PR queues per-request handlers FIFO so each response reaches the callback that sent it.The bug
api_clientkept a singlepending_callbackand one self-clearingon_http_response/on_http_errorpair on the transport, so only one request could safely be in flight. #7's "Leave Queue & Continue" guard leaves the queue and immediately replays the blocked action; when consumers guard lobby join with it, leave + join fire in the same tick. The join overwrites the leave's handler; the leave's response then runs the join's parser on the wrong body and errors (in game: "Failed to parse server response"); the handler clears itself, so the join's response is dropped and the leave callback never fires. The client is stranded; the bug is latent for any overlapping pair.The fix
client.luanow keeps_queue, a FIFO of{on_response, on_error}entries, plus a transport router that pops the front entry for each inbound event.Positional matching is correct because the MQTT worker (
mqtt_thread.lua) drainstx_channelserially and blocks on each request, so responses return in send order._enqueueis called immediately before each send, so queue order matches send order. Note_enqueuere-installs the router on every call; this is deliberate and idempotent (the closures only pop_queue).The second commit (2ff2289) documents the three invariants this relies on — serial worker (responses arrive in send order), exactly one terminal event per request (
request_with_retryis bounded), no in-place transport swap (reconnect builds a fresh client with an empty_queue) — and adds a tripwire: under those invariants an event with an empty queue is impossible, so the router warns loudly instead of misrouting silently.How to review
Review as one diff; the second commit only adds comments, the tripwire, and its test.
networking/api_client/client.lua(_install_router,_enqueue, and the shared_setup_http_callback/_setup_json_callbackmoved onto the queue). Start here.account.lua(discord link/unlink) andlobby.lua(set metadata, enable chat, send chat) were the inline single-slot call sites.unlink_discordandset_lobby_metadatacollapse onto_setup_json_callback; the other three keep their bespoke parsers, now passed to_enqueue, to preserve behavior. Nothing bypasses the queue.Tests
Standalone script from the repo root; not wired into CI. Green: overlapping leave + join each get their own response; an error pops only the front request; the empty-queue tripwire warns. Red control: a transcription of the old single-slot design reproduces the bug (leave callback never fires; join callback gets the leave body and errors).
Related