fix(discord): commit callback state after provider acceptance (#117417)

Co-authored-by: Peter Steinberger <steipete@macos.shared>

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<void>((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<void>((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<void>((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<void>((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<void>((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);

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<void> = 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<T>(operation: () => Promise<T>): Promise<T> {
+    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<unknown> {
-    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<unknown> {
@@ -184,6 +205,10 @@ class BaseInteraction {
   }
 
   async editReply(payload: MessagePayload): Promise<unknown> {
+    return await this.enqueueResponse(() => this.performReplyEdit(payload));
+  }
+
+  private async performReplyEdit(payload: MessagePayload): Promise<unknown> {
     const body = serializePayload(payload);
     const query = needsComponentsV2Query(body) ? { with_components: true } : undefined;
     const result = query
@@ -207,22 +232,21 @@ class BaseInteraction {
   }
 
   async deleteReply(): Promise<unknown> {
-    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<unknown> {
-    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<unknown> {
+    return await this.enqueueResponse(() => this.performFollowUp(payload));
+  }
+
+  private async performFollowUp(payload: MessagePayload): Promise<unknown> {
     const body = serializePayload(payload);
     return await createWebhookMessage(
       this.client.rest,