fix(line): harden outbound send behavior

src/line/send.test.ts

@@ -1,11 +1,228 @@
-import { describe, expect, it } from "vitest";
-import { createQuickReplyItems } from "./send.js";
+import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
 
-describe("createQuickReplyItems", () => {
-  it("limits items to 13 (LINE maximum)", () => {
-    const labels = Array.from({ length: 20 }, (_, i) => `Option ${i + 1}`);
-    const quickReply = createQuickReplyItems(labels);
+const {
+  pushMessageMock,
+  replyMessageMock,
+  showLoadingAnimationMock,
+  getProfileMock,
+  MessagingApiClientMock,
+  loadConfigMock,
+  resolveLineAccountMock,
+  resolveLineChannelAccessTokenMock,
+  recordChannelActivityMock,
+  logVerboseMock,
+} = vi.hoisted(() => {
+  const pushMessageMock = vi.fn();
+  const replyMessageMock = vi.fn();
+  const showLoadingAnimationMock = vi.fn();
+  const getProfileMock = vi.fn();
+  const MessagingApiClientMock = vi.fn(function () {
+    return {
+      pushMessage: pushMessageMock,
+      replyMessage: replyMessageMock,
+      showLoadingAnimation: showLoadingAnimationMock,
+      getProfile: getProfileMock,
+    };
+  });
+  const loadConfigMock = vi.fn(() => ({}));
+  const resolveLineAccountMock = vi.fn(() => ({ accountId: "default" }));
+  const resolveLineChannelAccessTokenMock = vi.fn(() => "line-token");
+  const recordChannelActivityMock = vi.fn();
+  const logVerboseMock = vi.fn();
+  return {
+    pushMessageMock,
+    replyMessageMock,
+    showLoadingAnimationMock,
+    getProfileMock,
+    MessagingApiClientMock,
+    loadConfigMock,
+    resolveLineAccountMock,
+    resolveLineChannelAccessTokenMock,
+    recordChannelActivityMock,
+    logVerboseMock,
+  };
+});
+
+vi.mock("@line/bot-sdk", () => ({
+  messagingApi: { MessagingApiClient: MessagingApiClientMock },
+}));
+
+vi.mock("../config/config.js", () => ({
+  loadConfig: loadConfigMock,
+}));
+
+vi.mock("./accounts.js", () => ({
+  resolveLineAccount: resolveLineAccountMock,
+}));
+
+vi.mock("./channel-access-token.js", () => ({
+  resolveLineChannelAccessToken: resolveLineChannelAccessTokenMock,
+}));
+
+vi.mock("../infra/channel-activity.js", () => ({
+  recordChannelActivity: recordChannelActivityMock,
+}));
+
+vi.mock("../globals.js", () => ({
+  logVerbose: logVerboseMock,
+}));
+
+let sendModule: typeof import("./send.js");
+
+describe("LINE send helpers", () => {
+  beforeAll(async () => {
+    sendModule = await import("./send.js");
+  });
+
+  beforeEach(() => {
+    pushMessageMock.mockReset();
+    replyMessageMock.mockReset();
+    showLoadingAnimationMock.mockReset();
+    getProfileMock.mockReset();
+    MessagingApiClientMock.mockClear();
+    loadConfigMock.mockReset();
+    resolveLineAccountMock.mockReset();
+    resolveLineChannelAccessTokenMock.mockReset();
+    recordChannelActivityMock.mockReset();
+    logVerboseMock.mockReset();
+
+    loadConfigMock.mockReturnValue({});
+    resolveLineAccountMock.mockReturnValue({ accountId: "default" });
+    resolveLineChannelAccessTokenMock.mockReturnValue("line-token");
+    pushMessageMock.mockResolvedValue({});
+    replyMessageMock.mockResolvedValue({});
+    showLoadingAnimationMock.mockResolvedValue({});
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it("limits quick reply items to 13", () => {
+    const labels = Array.from({ length: 20 }, (_, index) => `Option ${index + 1}`);
+    const quickReply = sendModule.createQuickReplyItems(labels);
 
     expect(quickReply.items).toHaveLength(13);
   });
+
+  it("pushes images via normalized LINE target", async () => {
+    const result = await sendModule.pushImageMessage(
+      "line:user:U123",
+      "https://example.com/original.jpg",
+      undefined,
+      { verbose: true },
+    );
+
+    expect(pushMessageMock).toHaveBeenCalledWith({
+      to: "U123",
+      messages: [
+        {
+          type: "image",
+          originalContentUrl: "https://example.com/original.jpg",
+          previewImageUrl: "https://example.com/original.jpg",
+        },
+      ],
+    });
+    expect(recordChannelActivityMock).toHaveBeenCalledWith({
+      channel: "line",
+      accountId: "default",
+      direction: "outbound",
+    });
+    expect(logVerboseMock).toHaveBeenCalledWith("line: pushed image to U123");
+    expect(result).toEqual({ messageId: "push", chatId: "U123" });
+  });
+
+  it("replies when reply token is provided", async () => {
+    const result = await sendModule.sendMessageLine("line:group:C1", "Hello", {
+      replyToken: "reply-token",
+      mediaUrl: "https://example.com/media.jpg",
+      verbose: true,
+    });
+
+    expect(replyMessageMock).toHaveBeenCalledTimes(1);
+    expect(pushMessageMock).not.toHaveBeenCalled();
+    expect(replyMessageMock).toHaveBeenCalledWith({
+      replyToken: "reply-token",
+      messages: [
+        {
+          type: "image",
+          originalContentUrl: "https://example.com/media.jpg",
+          previewImageUrl: "https://example.com/media.jpg",
+        },
+        {
+          type: "text",
+          text: "Hello",
+        },
+      ],
+    });
+    expect(logVerboseMock).toHaveBeenCalledWith("line: replied to C1");
+    expect(result).toEqual({ messageId: "reply", chatId: "C1" });
+  });
+
+  it("throws when push messages are empty", async () => {
+    await expect(sendModule.pushMessagesLine("U123", [])).rejects.toThrow(
+      "Message must be non-empty for LINE sends",
+    );
+  });
+
+  it("logs HTTP body when push fails", async () => {
+    const err = new Error("LINE push failed") as Error & {
+      status: number;
+      statusText: string;
+      body: string;
+    };
+    err.status = 400;
+    err.statusText = "Bad Request";
+    err.body = "invalid flex payload";
+    pushMessageMock.mockRejectedValueOnce(err);
+
+    await expect(
+      sendModule.pushMessagesLine("U999", [{ type: "text", text: "hello" }]),
+    ).rejects.toThrow("LINE push failed");
+
+    expect(logVerboseMock).toHaveBeenCalledWith(
+      "line: push message failed (400 Bad Request): invalid flex payload",
+    );
+  });
+
+  it("caches profile results by default", async () => {
+    getProfileMock.mockResolvedValue({
+      displayName: "Peter",
+      pictureUrl: "https://example.com/peter.jpg",
+    });
+
+    const first = await sendModule.getUserProfile("U-cache");
+    const second = await sendModule.getUserProfile("U-cache");
+
+    expect(first).toEqual({
+      displayName: "Peter",
+      pictureUrl: "https://example.com/peter.jpg",
+    });
+    expect(second).toEqual(first);
+    expect(getProfileMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("continues when loading animation is unsupported", async () => {
+    showLoadingAnimationMock.mockRejectedValueOnce(new Error("unsupported"));
+
+    await expect(sendModule.showLoadingAnimation("line:room:R1")).resolves.toBeUndefined();
+
+    expect(logVerboseMock).toHaveBeenCalledWith(
+      expect.stringContaining("line: loading animation failed (non-fatal)"),
+    );
+  });
+
+  it("pushes quick-reply text and caps to 13 buttons", async () => {
+    await sendModule.pushTextMessageWithQuickReplies(
+      "U-quick",
+      "Pick one",
+      Array.from({ length: 20 }, (_, index) => `Choice ${index + 1}`),
+    );
+
+    expect(pushMessageMock).toHaveBeenCalledTimes(1);
+    const firstCall = pushMessageMock.mock.calls[0] as [
+      { messages: Array<{ quickReply?: { items: unknown[] } }> },
+    ];
+    expect(firstCall[0].messages[0].quickReply?.items).toHaveLength(13);
+  });
 });

src/line/send.ts

@@ -32,6 +32,18 @@ interface LineSendOpts {
   replyToken?: string;
 }
 
+type LineClientOpts = Pick<LineSendOpts, "channelAccessToken" | "accountId">;
+type LinePushOpts = Pick<LineSendOpts, "channelAccessToken" | "accountId" | "verbose">;
+
+interface LinePushBehavior {
+  errorContext?: string;
+  verboseMessage?: (chatId: string, messageCount: number) => string;
+}
+
+interface LineReplyBehavior {
+  verboseMessage?: (messageCount: number) => string;
+}
+
 function normalizeTarget(to: string): string {
   const trimmed = to.trim();
   if (!trimmed) {
@@ -52,7 +64,7 @@ function normalizeTarget(to: string): string {
   return normalized;
 }
 
-function createLineMessagingClient(opts: { channelAccessToken?: string; accountId?: string }): {
+function createLineMessagingClient(opts: LineClientOpts): {
   account: ReturnType<typeof resolveLineAccount>;
   client: messagingApi.MessagingApiClient;
 } {
@@ -70,7 +82,7 @@ function createLineMessagingClient(opts: { channelAccessToken?: string; accountI
 
 function createLinePushContext(
   to: string,
-  opts: { channelAccessToken?: string; accountId?: string },
+  opts: LineClientOpts,
 ): {
   account: ReturnType<typeof resolveLineAccount>;
   client: messagingApi.MessagingApiClient;
@@ -126,23 +138,85 @@ function logLineHttpError(err: unknown, context: string): void {
   }
 }
 
+function recordLineOutboundActivity(accountId: string): void {
+  recordChannelActivity({
+    channel: "line",
+    accountId,
+    direction: "outbound",
+  });
+}
+
+async function pushLineMessages(
+  to: string,
+  messages: Message[],
+  opts: LinePushOpts = {},
+  behavior: LinePushBehavior = {},
+): Promise<LineSendResult> {
+  if (messages.length === 0) {
+    throw new Error("Message must be non-empty for LINE sends");
+  }
+
+  const { account, client, chatId } = createLinePushContext(to, opts);
+  const pushRequest = client.pushMessage({
+    to: chatId,
+    messages,
+  });
+
+  if (behavior.errorContext) {
+    const errorContext = behavior.errorContext;
+    await pushRequest.catch((err) => {
+      logLineHttpError(err, errorContext);
+      throw err;
+    });
+  } else {
+    await pushRequest;
+  }
+
+  recordLineOutboundActivity(account.accountId);
+
+  if (opts.verbose) {
+    const logMessage =
+      behavior.verboseMessage?.(chatId, messages.length) ??
+      `line: pushed ${messages.length} messages to ${chatId}`;
+    logVerbose(logMessage);
+  }
+
+  return {
+    messageId: "push",
+    chatId,
+  };
+}
+
+async function replyLineMessages(
+  replyToken: string,
+  messages: Message[],
+  opts: LinePushOpts = {},
+  behavior: LineReplyBehavior = {},
+): Promise<void> {
+  const { account, client } = createLineMessagingClient(opts);
+
+  await client.replyMessage({
+    replyToken,
+    messages,
+  });
+
+  recordLineOutboundActivity(account.accountId);
+
+  if (opts.verbose) {
+    logVerbose(
+      behavior.verboseMessage?.(messages.length) ??
+        `line: replied with ${messages.length} messages`,
+    );
+  }
+}
+
 export async function sendMessageLine(
   to: string,
   text: string,
   opts: LineSendOpts = {},
 ): Promise<LineSendResult> {
-  const cfg = loadConfig();
-  const account = resolveLineAccount({
-    cfg,
-    accountId: opts.accountId,
-  });
-  const token = resolveLineChannelAccessToken(opts.channelAccessToken, account);
   const chatId = normalizeTarget(to);
 
-  const client = new messagingApi.MessagingApiClient({
-    channelAccessToken: token,
-  });
-
   const messages: Message[] = [];
 
   // Add media if provided
@@ -161,21 +235,10 @@ export async function sendMessageLine(
 
   // Use reply if we have a reply token, otherwise push
   if (opts.replyToken) {
-    await client.replyMessage({
-      replyToken: opts.replyToken,
-      messages,
+    await replyLineMessages(opts.replyToken, messages, opts, {
+      verboseMessage: () => `line: replied to ${chatId}`,
     });
 
-    recordChannelActivity({
-      channel: "line",
-      accountId: account.accountId,
-      direction: "outbound",
-    });
-
-    if (opts.verbose) {
-      logVerbose(`line: replied to ${chatId}`);
-    }
-
     return {
       messageId: "reply",
       chatId,
@@ -183,25 +246,9 @@ export async function sendMessageLine(
   }
 
   // Push message (for proactive messaging)
-  await client.pushMessage({
-    to: chatId,
-    messages,
+  return pushLineMessages(chatId, messages, opts, {
+    verboseMessage: (resolvedChatId) => `line: pushed message to ${resolvedChatId}`,
   });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
-  });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed message to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 export async function pushMessageLine(
@@ -216,61 +263,19 @@ export async function pushMessageLine(
 export async function replyMessageLine(
   replyToken: string,
   messages: Message[],
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<void> {
-  const { account, client } = createLineMessagingClient(opts);
-
-  await client.replyMessage({
-    replyToken,
-    messages,
-  });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
-  });
-
-  if (opts.verbose) {
-    logVerbose(`line: replied with ${messages.length} messages`);
-  }
+  await replyLineMessages(replyToken, messages, opts);
 }
 
 export async function pushMessagesLine(
   to: string,
   messages: Message[],
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<LineSendResult> {
-  if (messages.length === 0) {
-    throw new Error("Message must be non-empty for LINE sends");
-  }
-
-  const { account, client, chatId } = createLinePushContext(to, opts);
-
-  await client
-    .pushMessage({
-      to: chatId,
-      messages,
-    })
-    .catch((err) => {
-      logLineHttpError(err, "push message");
-      throw err;
-    });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
+  return pushLineMessages(to, messages, opts, {
+    errorContext: "push message",
   });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed ${messages.length} messages to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 export function createFlexMessage(
@@ -291,31 +296,11 @@ export async function pushImageMessage(
   to: string,
   originalContentUrl: string,
   previewImageUrl?: string,
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<LineSendResult> {
-  const { account, client, chatId } = createLinePushContext(to, opts);
-
-  const imageMessage = createImageMessage(originalContentUrl, previewImageUrl);
-
-  await client.pushMessage({
-    to: chatId,
-    messages: [imageMessage],
+  return pushLineMessages(to, [createImageMessage(originalContentUrl, previewImageUrl)], opts, {
+    verboseMessage: (chatId) => `line: pushed image to ${chatId}`,
   });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
-  });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed image to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 /**
@@ -329,31 +314,11 @@ export async function pushLocationMessage(
     latitude: number;
     longitude: number;
   },
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<LineSendResult> {
-  const { account, client, chatId } = createLinePushContext(to, opts);
-
-  const locationMessage = createLocationMessage(location);
-
-  await client.pushMessage({
-    to: chatId,
-    messages: [locationMessage],
+  return pushLineMessages(to, [createLocationMessage(location)], opts, {
+    verboseMessage: (chatId) => `line: pushed location to ${chatId}`,
   });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
-  });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed location to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 /**
@@ -363,40 +328,18 @@ export async function pushFlexMessage(
   to: string,
   altText: string,
   contents: FlexContainer,
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<LineSendResult> {
-  const { account, client, chatId } = createLinePushContext(to, opts);
-
   const flexMessage: FlexMessage = {
     type: "flex",
     altText: altText.slice(0, 400), // LINE limit
     contents,
   };
 
-  await client
-    .pushMessage({
-      to: chatId,
-      messages: [flexMessage],
-    })
-    .catch((err) => {
-      logLineHttpError(err, "push flex message");
-      throw err;
-    });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
+  return pushLineMessages(to, [flexMessage], opts, {
+    errorContext: "push flex message",
+    verboseMessage: (chatId) => `line: pushed flex message to ${chatId}`,
   });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed flex message to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 /**
@@ -405,29 +348,11 @@ export async function pushFlexMessage(
 export async function pushTemplateMessage(
   to: string,
   template: TemplateMessage,
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<LineSendResult> {
-  const { account, client, chatId } = createLinePushContext(to, opts);
-
-  await client.pushMessage({
-    to: chatId,
-    messages: [template],
+  return pushLineMessages(to, [template], opts, {
+    verboseMessage: (chatId) => `line: pushed template message to ${chatId}`,
   });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
-  });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed template message to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 /**
@@ -437,31 +362,13 @@ export async function pushTextMessageWithQuickReplies(
   to: string,
   text: string,
   quickReplyLabels: string[],
-  opts: { channelAccessToken?: string; accountId?: string; verbose?: boolean } = {},
+  opts: LinePushOpts = {},
 ): Promise<LineSendResult> {
-  const { account, client, chatId } = createLinePushContext(to, opts);
-
   const message = createTextMessageWithQuickReplies(text, quickReplyLabels);
 
-  await client.pushMessage({
-    to: chatId,
-    messages: [message],
+  return pushLineMessages(to, [message], opts, {
+    verboseMessage: (chatId) => `line: pushed message with quick replies to ${chatId}`,
   });
-
-  recordChannelActivity({
-    channel: "line",
-    accountId: account.accountId,
-    direction: "outbound",
-  });
-
-  if (opts.verbose) {
-    logVerbose(`line: pushed message with quick replies to ${chatId}`);
-  }
-
-  return {
-    messageId: "push",
-    chatId,
-  };
 }
 
 /**
@@ -500,16 +407,7 @@ export async function showLoadingAnimation(
   chatId: string,
   opts: { channelAccessToken?: string; accountId?: string; loadingSeconds?: number } = {},
 ): Promise<void> {
-  const cfg = loadConfig();
-  const account = resolveLineAccount({
-    cfg,
-    accountId: opts.accountId,
-  });
-  const token = resolveLineChannelAccessToken(opts.channelAccessToken, account);
-
-  const client = new messagingApi.MessagingApiClient({
-    channelAccessToken: token,
-  });
+  const { client } = createLineMessagingClient(opts);
 
   try {
     await client.showLoadingAnimation({
@@ -540,16 +438,7 @@ export async function getUserProfile(
     }
   }
 
-  const cfg = loadConfig();
-  const account = resolveLineAccount({
-    cfg,
-    accountId: opts.accountId,
-  });
-  const token = resolveLineChannelAccessToken(opts.channelAccessToken, account);
-
-  const client = new messagingApi.MessagingApiClient({
-    channelAccessToken: token,
-  });
+  const { client } = createLineMessagingClient(opts);
 
   try {
     const profile = await client.getProfile(userId);