From 47be8b8e1069cd04310d5090da3566b8e05fc802 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 2 Aug 2026 11:30:37 -0700 Subject: [PATCH] fix(discord): commit callback state after provider acceptance (#117417) Co-authored-by: Peter Steinberger --- .../discord/src/internal/interactions.test.ts | 394 +++++++++++++++++- .../discord/src/internal/interactions.ts | 82 ++-- 2 files changed, 448 insertions(+), 28 deletions(-) diff --git a/extensions/discord/src/internal/interactions.test.ts b/extensions/discord/src/internal/interactions.test.ts index e31762120753..09bea6130358 100644 --- a/extensions/discord/src/internal/interactions.test.ts +++ b/extensions/discord/src/internal/interactions.test.ts @@ -8,7 +8,13 @@ import { } from "discord-api-types/v10"; import { describe, expect, it, vi } from "vitest"; import { Container, TextDisplay } from "./components.js"; -import { ModalInteraction, createInteraction, type RawInteraction } from "./interactions.js"; +import { + AutocompleteInteraction, + BaseComponentInteraction, + ModalInteraction, + createInteraction, + type RawInteraction, +} from "./interactions.js"; import { Message } from "./structures.js"; import { attachRestMock, @@ -19,6 +25,392 @@ import { } from "./test-builders.test-support.js"; describe("BaseInteraction", () => { + it.each([ + { + name: "command reply", + kind: "command", + operation: "reply", + callbackType: InteractionResponseType.ChannelMessageWithSource, + }, + { + name: "command defer", + kind: "command", + operation: "defer", + callbackType: InteractionResponseType.DeferredChannelMessageWithSource, + }, + { + name: "component acknowledgment", + kind: "component", + operation: "acknowledge", + callbackType: InteractionResponseType.DeferredMessageUpdate, + }, + { + name: "component update", + kind: "component", + operation: "update", + callbackType: InteractionResponseType.UpdateMessage, + }, + { + name: "component modal", + kind: "component", + operation: "show-modal", + callbackType: InteractionResponseType.Modal, + }, + { + name: "component activity", + kind: "component", + operation: "launch-activity", + callbackType: InteractionResponseType.LaunchActivity, + }, + { + name: "modal acknowledgment", + kind: "modal", + operation: "acknowledge", + callbackType: InteractionResponseType.DeferredMessageUpdate, + }, + { + name: "command autocomplete", + kind: "autocomplete", + operation: "autocomplete", + callbackType: InteractionResponseType.ApplicationCommandAutocompleteResult, + }, + ] as const)("keeps a rejected $name eligible for its initial callback", async (testCase) => { + const failure = new Error("Discord rejected the interaction callback"); + const post = vi.fn().mockRejectedValueOnce(failure).mockResolvedValue(undefined); + const patch = vi.fn(async () => undefined); + const client = createInternalTestClient(); + attachRestMock(client, { patch, post }); + const identity = { id: "interaction1", token: "token1" }; + const payload = + testCase.kind === "component" + ? createInternalComponentInteractionPayload(identity) + : testCase.kind === "modal" + ? createInternalModalInteractionPayload(identity) + : createInternalInteractionPayload({ + ...identity, + ...(testCase.kind === "autocomplete" + ? { type: InteractionType.ApplicationCommandAutocomplete } + : {}), + }); + const interaction = createInteraction(client, payload); + + const sendInitialCallback = () => { + switch (testCase.operation) { + case "reply": + return interaction.reply("first"); + case "defer": + return interaction.defer(); + case "acknowledge": + return interaction.acknowledge(); + case "update": + if (!(interaction instanceof BaseComponentInteraction)) { + throw new Error("expected a component interaction"); + } + return interaction.update("first"); + case "show-modal": + if (!(interaction instanceof BaseComponentInteraction)) { + throw new Error("expected a component interaction"); + } + return interaction.showModal({ serialize: () => ({ title: "Choose" }) }); + case "launch-activity": + if (!(interaction instanceof BaseComponentInteraction)) { + throw new Error("expected a component interaction"); + } + return interaction.launchActivity(); + case "autocomplete": + if (!(interaction instanceof AutocompleteInteraction)) { + throw new Error("expected an autocomplete interaction"); + } + return interaction.respond([{ name: "Choice", value: "choice" }]); + default: + throw new Error("expected a supported interaction callback"); + } + }; + + await expect(sendInitialCallback()).rejects.toBe(failure); + expect(interaction.responseState).toBe("unacknowledged"); + expect(interaction.acknowledged).toBe(false); + + if (testCase.operation === "autocomplete") { + await sendInitialCallback(); + } else { + await interaction.reply("recovered"); + } + + expect(post).toHaveBeenNthCalledWith( + 1, + "/interactions/interaction1/token1/callback", + expect.objectContaining({ body: expect.objectContaining({ type: testCase.callbackType }) }), + ); + expect(post).toHaveBeenNthCalledWith(2, "/interactions/interaction1/token1/callback", { + body: + testCase.operation === "autocomplete" + ? { + type: InteractionResponseType.ApplicationCommandAutocompleteResult, + data: { choices: [{ name: "Choice", value: "choice" }] }, + } + : { + type: InteractionResponseType.ChannelMessageWithSource, + data: { content: "recovered" }, + }, + }); + expect(patch).not.toHaveBeenCalled(); + expect(interaction.responseState).toBe("replied"); + }); + + it("waits for provider acceptance before recording a deferred interaction", async () => { + let acceptCallback!: () => void; + const accepted = new Promise((resolve) => { + acceptCallback = resolve; + }); + const post = vi.fn(() => accepted); + const client = createInternalTestClient(); + attachRestMock(client, { post }); + const interaction = createInteraction( + client, + createInternalInteractionPayload({ id: "interaction1", token: "token1" }), + ); + + const pending = interaction.defer(); + const stateBeforeAcceptance = interaction.responseState; + acceptCallback(); + await pending; + + expect(stateBeforeAcceptance).toBe("unacknowledged"); + expect(interaction.responseState).toBe("deferred"); + expect(interaction.acknowledged).toBe(true); + }); + + it.each([ + { first: "accepted", nextRoute: "/webhooks/app1/token1" }, + { first: "rejected", nextRoute: "/interactions/interaction1/token1/callback" }, + ] as const)( + "serializes concurrent replies after the first callback is $first", + async (testCase) => { + let acceptFirst!: () => void; + let rejectFirst!: (error: Error) => void; + const firstResponse = new Promise((resolve, reject) => { + acceptFirst = resolve; + rejectFirst = reject; + }); + const post = vi + .fn() + .mockImplementationOnce(() => firstResponse) + .mockResolvedValue(undefined); + const client = createInternalTestClient(); + attachRestMock(client, { post }); + const interaction = createInteraction( + client, + createInternalInteractionPayload({ id: "interaction1", token: "token1" }), + ); + + const first = interaction.reply("first"); + await vi.waitFor(() => expect(post).toHaveBeenCalledOnce()); + const second = interaction.reply("second"); + const callsBeforeAcceptance = post.mock.calls.length; + const settled = Promise.allSettled([first, second]); + if (testCase.first === "accepted") { + acceptFirst(); + } else { + rejectFirst(new Error("Discord rejected the first callback")); + } + + const [firstResult, secondResult] = await settled; + expect(callsBeforeAcceptance).toBe(1); + expect(firstResult.status).toBe(testCase.first === "accepted" ? "fulfilled" : "rejected"); + expect(secondResult.status).toBe("fulfilled"); + expect(post).toHaveBeenNthCalledWith(1, "/interactions/interaction1/token1/callback", { + body: { + type: InteractionResponseType.ChannelMessageWithSource, + data: { content: "first" }, + }, + }); + if (testCase.first === "accepted") { + expect(post).toHaveBeenNthCalledWith( + 2, + testCase.nextRoute, + { body: { content: "second" } }, + undefined, + ); + } else { + expect(post).toHaveBeenNthCalledWith(2, testCase.nextRoute, { + body: { + type: InteractionResponseType.ChannelMessageWithSource, + data: { content: "second" }, + }, + }); + } + expect(interaction.responseState).toBe("replied"); + }, + ); + + it("waits for the accepted initial callback before an explicit follow-up", async () => { + let acceptFirst!: () => void; + const firstResponse = new Promise((resolve) => { + acceptFirst = resolve; + }); + const post = vi + .fn() + .mockImplementationOnce(() => firstResponse) + .mockResolvedValue(undefined); + const client = createInternalTestClient(); + attachRestMock(client, { post }); + const interaction = createInteraction( + client, + createInternalInteractionPayload({ id: "interaction1", token: "token1" }), + ); + + const initial = interaction.reply("first"); + await vi.waitFor(() => expect(post).toHaveBeenCalledOnce()); + const followUp = interaction.followUp("second"); + const callsBeforeAcceptance = post.mock.calls.length; + acceptFirst(); + await Promise.all([initial, followUp]); + + expect(callsBeforeAcceptance).toBe(1); + expect(post).toHaveBeenNthCalledWith( + 2, + "/webhooks/app1/token1", + { body: { content: "second" } }, + undefined, + ); + }); + + it.each([ + { kind: "command", operation: "defer" }, + { kind: "component", operation: "defer" }, + { kind: "component", operation: "acknowledge" }, + { kind: "component", operation: "update" }, + { kind: "component", operation: "show-modal" }, + { kind: "component", operation: "launch-activity" }, + { kind: "modal", operation: "defer" }, + { kind: "modal", operation: "acknowledge" }, + { kind: "autocomplete", operation: "autocomplete" }, + ] as const)( + "rejects a second initial $kind $operation without calling Discord", + async (testCase) => { + const post = vi.fn(async () => undefined); + const client = createInternalTestClient(); + attachRestMock(client, { post }); + const identity = { id: "interaction1", token: "token1" }; + const raw = + testCase.kind === "component" + ? createInternalComponentInteractionPayload(identity) + : testCase.kind === "modal" + ? createInternalModalInteractionPayload(identity) + : createInternalInteractionPayload({ + ...identity, + ...(testCase.kind === "autocomplete" + ? { type: InteractionType.ApplicationCommandAutocomplete } + : {}), + }); + const interaction = createInteraction(client, raw); + const first = + testCase.kind === "autocomplete" && interaction instanceof AutocompleteInteraction + ? interaction.respond([{ name: "First", value: "first" }]) + : interaction.reply("first"); + const next = () => { + switch (testCase.operation) { + case "defer": + return interaction.defer(); + case "acknowledge": + return interaction.acknowledge(); + case "update": + if (!(interaction instanceof BaseComponentInteraction)) { + throw new Error("expected a component interaction"); + } + return interaction.update("second"); + case "show-modal": + if (!(interaction instanceof BaseComponentInteraction)) { + throw new Error("expected a component interaction"); + } + return interaction.showModal({ serialize: () => ({ title: "Choose" }) }); + case "launch-activity": + if (!(interaction instanceof BaseComponentInteraction)) { + throw new Error("expected a component interaction"); + } + return interaction.launchActivity(); + case "autocomplete": + if (!(interaction instanceof AutocompleteInteraction)) { + throw new Error("expected an autocomplete interaction"); + } + return interaction.respond([{ name: "Second", value: "second" }]); + default: + throw new Error("expected an initial interaction callback"); + } + }; + + const [firstResult, secondResult] = await Promise.allSettled([first, next()]); + expect(firstResult.status).toBe("fulfilled"); + expect(secondResult.status).toBe("rejected"); + if (secondResult.status === "rejected") { + expect(secondResult.reason).toMatchObject({ + message: expect.stringMatching(/already.*acknowledg/i), + }); + } + expect(post).toHaveBeenCalledOnce(); + expect(interaction.acknowledged).toBe(true); + }, + ); + + it("waits for the initial response before fetching the original reply", async () => { + let acceptFirst!: () => void; + const firstResponse = new Promise((resolve) => { + acceptFirst = resolve; + }); + const post = vi.fn(() => firstResponse); + const get = vi.fn(async () => ({ id: "message1" })); + const client = createInternalTestClient(); + attachRestMock(client, { get, post }); + const interaction = createInteraction( + client, + createInternalInteractionPayload({ id: "interaction1", token: "token1" }), + ); + + const initial = interaction.reply("first"); + await vi.waitFor(() => expect(post).toHaveBeenCalledOnce()); + const fetched = interaction.fetchReply(); + const fetchesBeforeAcceptance = get.mock.calls.length; + acceptFirst(); + + await expect(Promise.all([initial, fetched])).resolves.toEqual([undefined, { id: "message1" }]); + expect(fetchesBeforeAcceptance).toBe(0); + expect(get).toHaveBeenCalledWith("/webhooks/app1/token1/messages/%40original"); + }); + + it("keeps response transactions independent for different interactions", async () => { + let acceptFirst!: () => void; + const firstResponse = new Promise((resolve) => { + acceptFirst = resolve; + }); + const post = vi + .fn() + .mockImplementationOnce(() => firstResponse) + .mockResolvedValue(undefined); + const client = createInternalTestClient(); + attachRestMock(client, { post }); + const firstInteraction = createInteraction( + client, + createInternalInteractionPayload({ id: "interaction1", token: "token1" }), + ); + const secondInteraction = createInteraction( + client, + createInternalInteractionPayload({ id: "interaction2", token: "token2" }), + ); + + const first = firstInteraction.reply("first"); + await vi.waitFor(() => expect(post).toHaveBeenCalledOnce()); + await secondInteraction.reply("second"); + expect(firstInteraction.responseState).toBe("unacknowledged"); + expect(secondInteraction.responseState).toBe("replied"); + acceptFirst(); + await first; + + expect(post).toHaveBeenNthCalledWith(2, "/interactions/interaction2/token2/callback", { + body: { type: InteractionResponseType.ChannelMessageWithSource, data: { content: "second" } }, + }); + }); + it("edits the original interaction response after defer", async () => { const post = vi.fn(async () => undefined); const patch = vi.fn(async () => undefined); diff --git a/extensions/discord/src/internal/interactions.ts b/extensions/discord/src/internal/interactions.ts index 22cfa27871e4..88443c743be1 100644 --- a/extensions/discord/src/internal/interactions.ts +++ b/extensions/discord/src/internal/interactions.ts @@ -120,6 +120,7 @@ class BaseInteraction { readonly channel: DiscordChannel | null; message: Message | null = null; private readonly response = new InteractionResponseController(); + private pendingResponse: Promise = Promise.resolve(); constructor( public client: InteractionClient, @@ -148,28 +149,48 @@ class BaseInteraction { this.response.state = nextState; } - protected async callback(type: InteractionResponseType, data?: unknown) { - this.response.recordCallback(type); - return await createInteractionCallback( + private enqueueResponse(operation: () => Promise): Promise { + const result = this.pendingResponse.then(operation); + // Keep the per-interaction queue live after provider rejection without swallowing it for callers. + this.pendingResponse = result.then( + () => undefined, + () => undefined, + ); + return result; + } + + private async performCallback(type: InteractionResponseType, data?: unknown) { + if (this.response.acknowledged) { + throw new Error("Discord interaction has already been acknowledged."); + } + const result = await createInteractionCallback( this.client.rest, this.id, this.token, data === undefined ? { type } : { type, data }, ); + this.response.recordCallback(type); + return result; + } + + protected async callback(type: InteractionResponseType, data?: unknown) { + return await this.enqueueResponse(() => this.performCallback(type, data)); } async reply(payload: MessagePayload): Promise { - const action = this.response.nextReplyAction(); - if (action === "edit") { - return await this.editReply(payload); - } - if (action === "follow-up") { - return await this.followUp(payload); - } - return await this.callback( - InteractionResponseType.ChannelMessageWithSource, - serializePayload(payload), - ); + return await this.enqueueResponse(async () => { + const action = this.response.nextReplyAction(); + if (action === "edit") { + return await this.performReplyEdit(payload); + } + if (action === "follow-up") { + return await this.performFollowUp(payload); + } + return await this.performCallback( + InteractionResponseType.ChannelMessageWithSource, + serializePayload(payload), + ); + }); } async defer(options?: { ephemeral?: boolean }): Promise { @@ -184,6 +205,10 @@ class BaseInteraction { } async editReply(payload: MessagePayload): Promise { + return await this.enqueueResponse(() => this.performReplyEdit(payload)); + } + + private async performReplyEdit(payload: MessagePayload): Promise { const body = serializePayload(payload); const query = needsComponentsV2Query(body) ? { with_components: true } : undefined; const result = query @@ -207,22 +232,21 @@ class BaseInteraction { } async deleteReply(): Promise { - const result = await deleteWebhookMessage( - this.client.rest, - this.client.options.clientId, - this.token, - "@original", - ); - this.response.recordReplyDelete(); - return result; + return await this.enqueueResponse(async () => { + const result = await deleteWebhookMessage( + this.client.rest, + this.client.options.clientId, + this.token, + "@original", + ); + this.response.recordReplyDelete(); + return result; + }); } async fetchReply(): Promise { - return await getWebhookMessage( - this.client.rest, - this.client.options.clientId, - this.token, - "@original", + return await this.enqueueResponse(() => + getWebhookMessage(this.client.rest, this.client.options.clientId, this.token, "@original"), ); } @@ -237,6 +261,10 @@ class BaseInteraction { } async followUp(payload: MessagePayload): Promise { + return await this.enqueueResponse(() => this.performFollowUp(payload)); + } + + private async performFollowUp(payload: MessagePayload): Promise { const body = serializePayload(payload); return await createWebhookMessage( this.client.rest,