mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(line): surface partial delivery when rich or media send fails alongside text (#100996)
* fix(line): surface partial delivery when rich or media send fails alongside text * fix(line): harden partial delivery result * test(line): preserve frozen error type * chore: keep release changelog owner-only --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
co-authored by
Peter Steinberger
parent
962d1096d4
commit
f859d78b61
@@ -248,6 +248,103 @@ describe("deliverLineAutoReply", () => {
|
||||
expect(pushOrder).toBeLessThan(replyOrder);
|
||||
});
|
||||
|
||||
it("surfaces a visible partial delivery when a rich bubble fails alongside quick-reply text", async () => {
|
||||
// Quick replies attach to the trailing text bubble, so the flex/media send
|
||||
// (pushMessagesLine) runs first. If it fails, the text still reaches the
|
||||
// user, but the loss must be reported instead of a silent full success.
|
||||
const createTextMessageWithQuickReplies = vi.fn((text: string) => ({
|
||||
type: "text" as const,
|
||||
text,
|
||||
quickReply: { items: ["A"] },
|
||||
}));
|
||||
const lineData = {
|
||||
flexMessage: { altText: "Card", contents: { type: "bubble" } },
|
||||
quickReplies: ["A"],
|
||||
};
|
||||
const failingPush = vi.fn(async () => {
|
||||
throw new Error("push failed");
|
||||
});
|
||||
const { deps, replyMessageLine } = createDeps({
|
||||
createTextMessageWithQuickReplies:
|
||||
createTextMessageWithQuickReplies as LineAutoReplyDeps["createTextMessageWithQuickReplies"],
|
||||
pushMessagesLine: failingPush as LineAutoReplyDeps["pushMessagesLine"],
|
||||
});
|
||||
|
||||
const result = await deliverLineAutoReply({
|
||||
...baseDeliveryParams,
|
||||
payload: { text: "hello", channelData: { line: lineData } },
|
||||
lineData,
|
||||
deps,
|
||||
});
|
||||
|
||||
// The partial failure is returned (not thrown) so the caller can adopt the
|
||||
// consumed reply-token state before surfacing it. visibleReplySent is the
|
||||
// signal dispatch uses to keep the sent text yet still report the failure.
|
||||
expect(result).toMatchObject({
|
||||
status: "partial",
|
||||
error: { sentBeforeError: true, visibleReplySent: true },
|
||||
});
|
||||
expect(result.replyTokenUsed).toBe(true);
|
||||
// Text still reached the user over the reply token despite the rich failure.
|
||||
expect(replyMessageLine).toHaveBeenCalledTimes(1);
|
||||
expect(failingPush).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("surfaces a visible partial delivery when a rich bubble fails after text without quick replies", async () => {
|
||||
// Without quick replies the text goes first and the rich bubble follows; a
|
||||
// failed rich push must surface the same visible partial delivery so the
|
||||
// sibling path stays consistent with the quick-reply branch.
|
||||
const lineData = {
|
||||
flexMessage: { altText: "Card", contents: { type: "bubble" } },
|
||||
};
|
||||
const failingPush = vi.fn(async () => {
|
||||
throw new Error("push failed");
|
||||
});
|
||||
const { deps, replyMessageLine } = createDeps({
|
||||
pushMessagesLine: failingPush as LineAutoReplyDeps["pushMessagesLine"],
|
||||
});
|
||||
|
||||
const result = await deliverLineAutoReply({
|
||||
...baseDeliveryParams,
|
||||
payload: { text: "hello", channelData: { line: lineData } },
|
||||
lineData,
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
status: "partial",
|
||||
error: { sentBeforeError: true, visibleReplySent: true },
|
||||
});
|
||||
expect(result.replyTokenUsed).toBe(true);
|
||||
expect(replyMessageLine).toHaveBeenCalledTimes(1);
|
||||
expect(failingPush).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("wraps a non-extensible rich failure without losing visible-send evidence", async () => {
|
||||
const lineData = {
|
||||
flexMessage: { altText: "Card", contents: { type: "bubble" } },
|
||||
};
|
||||
const frozenError = new Error("push failed");
|
||||
Object.freeze(frozenError);
|
||||
const { deps } = createDeps({
|
||||
pushMessagesLine: vi.fn(async () => {
|
||||
throw frozenError;
|
||||
}) as LineAutoReplyDeps["pushMessagesLine"],
|
||||
});
|
||||
|
||||
const result = await deliverLineAutoReply({
|
||||
...baseDeliveryParams,
|
||||
payload: { text: "hello", channelData: { line: lineData } },
|
||||
lineData,
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
status: "partial",
|
||||
error: { sentBeforeError: true, visibleReplySent: true, cause: frozenError },
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to push when reply token delivery fails", async () => {
|
||||
const lineData = {
|
||||
flexMessage: { altText: "Card", contents: { type: "bubble" } },
|
||||
|
||||
@@ -43,6 +43,20 @@ export type LineAutoReplyDeps = {
|
||||
| "onReplyError"
|
||||
>;
|
||||
|
||||
export type LineAutoReplyDeliveryResult =
|
||||
| { status: "delivered"; replyTokenUsed: boolean }
|
||||
| { status: "partial"; replyTokenUsed: boolean; error: Error };
|
||||
|
||||
function markLineVisibleDeliveryError(error: unknown): Error {
|
||||
if (error instanceof Error && Object.isExtensible(error)) {
|
||||
Object.assign(error, { sentBeforeError: true, visibleReplySent: true });
|
||||
return error;
|
||||
}
|
||||
const visibleError = new Error("LINE rich or media message send failed", { cause: error });
|
||||
Object.assign(visibleError, { sentBeforeError: true, visibleReplySent: true });
|
||||
return visibleError;
|
||||
}
|
||||
|
||||
export async function deliverLineAutoReply(params: {
|
||||
payload: ReplyPayload;
|
||||
lineData: LineChannelData;
|
||||
@@ -53,7 +67,7 @@ export async function deliverLineAutoReply(params: {
|
||||
cfg: OpenClawConfig;
|
||||
textLimit: number;
|
||||
deps: LineAutoReplyDeps;
|
||||
}): Promise<{ replyTokenUsed: boolean }> {
|
||||
}): Promise<LineAutoReplyDeliveryResult> {
|
||||
const { payload, lineData, replyToken, accountId, to, textLimit, deps } = params;
|
||||
let replyTokenUsed = params.replyTokenUsed;
|
||||
|
||||
@@ -141,11 +155,17 @@ export async function deliverLineAutoReply(params: {
|
||||
|
||||
if (chunks.length > 0) {
|
||||
const hasRichOrMedia = richMessages.length > 0 || mediaMessages.length > 0;
|
||||
if (hasQuickReplies && hasRichOrMedia) {
|
||||
// Quick replies attach to the trailing message, so when both are present the
|
||||
// rich/media bubbles must go out before the quick-reply text. Capture a
|
||||
// failure instead of swallowing it: the text still sends below, but a lost
|
||||
// rich/media bubble must surface as a partial delivery, not silent success.
|
||||
const sendRichBeforeText = hasQuickReplies && hasRichOrMedia;
|
||||
let richMediaError: unknown;
|
||||
if (sendRichBeforeText) {
|
||||
try {
|
||||
await sendLineMessages([...richMessages, ...mediaMessages], false);
|
||||
} catch (err) {
|
||||
deps.onReplyError?.(err);
|
||||
richMediaError = err;
|
||||
}
|
||||
}
|
||||
const { replyTokenUsed: nextReplyTokenUsed } = await deps.sendLineReplyChunks({
|
||||
@@ -162,12 +182,25 @@ export async function deliverLineAutoReply(params: {
|
||||
createTextMessageWithQuickReplies: deps.createTextMessageWithQuickReplies,
|
||||
});
|
||||
replyTokenUsed = nextReplyTokenUsed;
|
||||
if (!hasQuickReplies || !hasRichOrMedia) {
|
||||
await sendLineMessages(richMessages, false);
|
||||
if (mediaMessages.length > 0) {
|
||||
await sendLineMessages(mediaMessages, false);
|
||||
if (!sendRichBeforeText) {
|
||||
try {
|
||||
await sendLineMessages(richMessages, false);
|
||||
if (mediaMessages.length > 0) {
|
||||
await sendLineMessages(mediaMessages, false);
|
||||
}
|
||||
} catch (err) {
|
||||
richMediaError = err;
|
||||
}
|
||||
}
|
||||
if (richMediaError !== undefined) {
|
||||
// Preserve both generic send evidence and foreground visibility: downstream
|
||||
// callers must surface the failure without retrying text the user already saw.
|
||||
return {
|
||||
status: "partial",
|
||||
replyTokenUsed,
|
||||
error: markLineVisibleDeliveryError(richMediaError),
|
||||
};
|
||||
}
|
||||
} else {
|
||||
const combined = [...richMessages, ...mediaMessages];
|
||||
if (hasQuickReplies && combined.length === 0) {
|
||||
@@ -200,5 +233,5 @@ export async function deliverLineAutoReply(params: {
|
||||
}
|
||||
}
|
||||
|
||||
return { replyTokenUsed };
|
||||
return { status: "delivered", replyTokenUsed };
|
||||
}
|
||||
|
||||
@@ -258,7 +258,7 @@ export async function monitorLineProvider(
|
||||
}).catch(() => {});
|
||||
}
|
||||
|
||||
const { replyTokenUsed: nextReplyTokenUsed } = await deliverLineAutoReply({
|
||||
const deliveryResult = await deliverLineAutoReply({
|
||||
payload,
|
||||
lineData,
|
||||
to: ctxPayload.From,
|
||||
@@ -288,7 +288,16 @@ export async function monitorLineProvider(
|
||||
},
|
||||
},
|
||||
});
|
||||
replyTokenUsed = nextReplyTokenUsed;
|
||||
replyTokenUsed = deliveryResult.replyTokenUsed;
|
||||
|
||||
if (deliveryResult.status === "partial") {
|
||||
// Text reached the user but a rich/media bubble did not.
|
||||
// Surface the tagged partial failure after adopting the
|
||||
// consumed reply-token state so later blocks in this turn
|
||||
// route correctly; recordChannelRuntimeState is skipped
|
||||
// because this delivery was not a clean success.
|
||||
throw deliveryResult.error;
|
||||
}
|
||||
|
||||
recordChannelRuntimeState({
|
||||
channel: "line",
|
||||
|
||||
Reference in New Issue
Block a user