Skip to content

fix(discord): forward the gateway packet as it arrived on the wire - #923

Open
amitvijapur wants to merge 3 commits into
vercel:mainfrom
amitvijapur:fix/discord-gateway-packet-snapshot
Open

amitvijapur wants to merge 3 commits into
vercel:mainfrom
amitvijapur:fix/discord-gateway-packet-snapshot

Conversation

@amitvijapur

Copy link
Copy Markdown
Contributor

Relates to #912.

What this changes

runGatewayListener's raw handler forwards the packet after resolving the message's channel. That lookup yields the event loop, and discord.js emits raw and calls handlePacket in the same synchronous turn, so it patches the very object we are about to forward. Message.js does:

this.guild.members._add(Object.assign(data.member, { user: this.author }));

Object.assign writes into the caller's data.member. The webhook therefore receives a member.user field Discord never sent on the wire.

This snapshots packet.d before the first await, so the forwarded payload is the wire packet regardless of what runs in between.

A correction to the issue, since this PR does not claim what it claims

I could not reproduce the circular-structure throw #912 describes, and I would rather say so than quietly ship a fix under a rationale I cannot support. Detail is in my comment on the issue, but briefly: this.author is a User, not a back-reference into d. It inherits toJSON from Base, whose constructor defines client non-enumerable, so JSON.stringify gets a flat object. A shared reference is not a cycle, and I found no path in discord.js@14.25.1 that writes a genuine back-edge into the packet.

So this PR is scoped to what is demonstrable: payload fidelity, not a dropped message. The reporter's stack trace is real, and the adapter declares ^14.25.1, so a deployment can resolve a different 14.x. If a version does produce a cycle, this same snapshot prevents it, because the clone runs before any mutation can land.

One thing in #912 that is understated rather than wrong: it scopes the problem to unregistered channels on the basis that registered ones skip the await. They do not. forwardGatewayEvent awaits resolveBotToken() before serializing on every path, and botToken is documented as accepting an async resolver. Any await is enough for the mutation to land first, so every forwarded event is affected, not only the channel-lookup path.

Why the snapshot, and why at the top

Two narrower options do not work:

  • Cloning only inside the channels.fetch branch misses the unconditional resolveBotToken() await inside forwardGatewayEvent. The top of the handler is the only placement before every yield.
  • A cycle-safe JSON.stringify replacer would change the shape sent to webhook consumers. The snapshot preserves the wire payload exactly.

Reassigning message to read from the snapshot is required, not a drive-by: the thread branch spreads { ...message }, so leaving it bound to packet.d would pull the mutated original back in.

structuredClone is already the pattern here, used in the Telegram, WhatsApp, Twilio and X adapters. It runs on every packet now, but every forwarded event already performs an HTTP POST, so the clone is not the cost. It cannot throw in practice: packet.d is decoded wire JSON, and the clone runs before discord.js can put a class instance on it.

Verification

  • @chat-adapter/discord suite: 295 passed.
  • tsc --noEmit clean.
  • The new test is load-bearing: reverting structuredClone to packet.d fails it, and it models the real Object.assign mutation rather than a synthetic one.

Disclosure: written with AI assistance, reviewed by me before opening.

discord.js emits `raw` and processes the packet in the same synchronous
turn, so once the handler yields on the channel lookup it patches the same
object in place. The webhook then received a `member.user` field Discord
never sent. Snapshot the packet before the first await.
@amitvijapur
amitvijapur requested a review from a team as a code owner September 11, 2026 12:22
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

@amitvijapur is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

…acket-snapshot

Forwarding moved behind enqueueOrderedForward on main (vercel#946, vercel#927), which
defers the task a microtask even on an empty queue, so the snapshot now
happens in the raw handler before the enqueue and the clone is what gets
forwarded.
@amitvijapur

Copy link
Copy Markdown
Contributor Author

Merged main. #946 and #927 moved the forwarding behind enqueueOrderedForward, which runs the task through previous.then(run, run), so for any packet with a channel_id it starts a microtask after the raw listener returns even when the queue is empty. That is after discord.js has already patched the packet (WebSocketManager.js emits raw at line 233 and handles the packet at line 236 of the same function, 14.25.1), so the snapshot now happens in the raw handler before the enqueue and the clone is what gets forwarded.

The test now patches the packet right after invoking the listener rather than inside the channel fetch, which matches that ordering. With the clone moved into forwardRawGatewayPacket it fails on the extra member.user; with the clone before the enqueue it passes. 304 tests, tsc --noEmit and ultracite check are clean.

This branch has not been deployed

No deployments
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.

1 participant