fix(discord): forward the gateway packet as it arrived on the wire - #923
amitvijapur wants to merge 3 commits into
Conversation
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 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.
|
Merged 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 |
Relates to #912.
What this changes
runGatewayListener'srawhandler forwards the packet after resolving the message's channel. That lookup yields the event loop, and discord.js emitsrawand callshandlePacketin the same synchronous turn, so it patches the very object we are about to forward.Message.jsdoes:Object.assignwrites into the caller'sdata.member. The webhook therefore receives amember.userfield Discord never sent on the wire.This snapshots
packet.dbefore the firstawait, 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.authoris aUser, not a back-reference intod. It inheritstoJSONfromBase, whose constructor definesclientnon-enumerable, soJSON.stringifygets a flat object. A shared reference is not a cycle, and I found no path indiscord.js@14.25.1that 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.forwardGatewayEventawaitsresolveBotToken()before serializing on every path, andbotTokenis documented as accepting an async resolver. Anyawaitis 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:
channels.fetchbranch misses the unconditionalresolveBotToken()await insideforwardGatewayEvent. The top of the handler is the only placement before every yield.JSON.stringifyreplacer would change the shape sent to webhook consumers. The snapshot preserves the wire payload exactly.Reassigning
messageto read from the snapshot is required, not a drive-by: the thread branch spreads{ ...message }, so leaving it bound topacket.dwould pull the mutated original back in.structuredCloneis 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.dis decoded wire JSON, and the clone runs before discord.js can put a class instance on it.Verification
@chat-adapter/discordsuite: 295 passed.tsc --noEmitclean.structuredClonetopacket.dfails it, and it models the realObject.assignmutation rather than a synthetic one.Disclosure: written with AI assistance, reviewed by me before opening.