From 4812af4e637e4d0490af21239d5f159ca802611b Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Wed, 5 Aug 2026 20:29:00 +0700 Subject: [PATCH] fix(chatwoot-adapter): tell the operator why a relay failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A relay that could not reach Chatwoot recorded its reason only in the host log, which the dashboard does not render. The health check reported bare counts, so "1 dead-lettered after 5 attempts" was the whole of what an operator could see — enough to know something was wrong, not enough to act. The health check now carries the last relay error alongside the counts, and only while something is failing, so a healthy plugin does not display a stale one. Also rejects a baseUrl carrying a path at config-read time. The client appends /api/v1/accounts/ to it, so a URL copied from the dashboard's address bar enabled cleanly and then 404'd every request, with the failure looking identical to a wrong token or an unreachable host. --- chatwoot-adapter/CHANGELOG.md | 11 ++++++++++ chatwoot-adapter/backfill.test.ts | 1 + chatwoot-adapter/echo-loop.test.ts | 3 +++ chatwoot-adapter/inbound.test.ts | 1 + chatwoot-adapter/inbound.ts | 1 + chatwoot-adapter/index.test.ts | 33 ++++++++++++++++++++++++++++++ chatwoot-adapter/index.ts | 13 ++++++++++++ chatwoot-adapter/relay.ts | 4 ++++ chatwoot-adapter/sent.test.ts | 1 + 9 files changed, 68 insertions(+) diff --git a/chatwoot-adapter/CHANGELOG.md b/chatwoot-adapter/CHANGELOG.md index 20c141b..46d748d 100644 --- a/chatwoot-adapter/CHANGELOG.md +++ b/chatwoot-adapter/CHANGELOG.md @@ -36,6 +36,17 @@ All notable changes to the Chatwoot Adapter plugin are documented here. The form the duplicate contacts in Chatwoot remains safe, as before. A contact already keyed to a WhatsApp JID is adopted but never re-keyed, so a chat seen under both JID forms cannot flip the contact back and forth. +- **A failing relay now says why, on the plugin's health check.** The reason a message did not reach + Chatwoot — the API status and response body, a refused private address, a certificate error — was + written to the host's log and nowhere else, while the health check reported only counts. An operator + saw "1 dead-lettered after 5 attempts" with no way to find out what went wrong. The health check now + appends `last error: …` whenever something is actually failing. +- **A `baseUrl` with a path is rejected when the settings are saved, instead of failing on every + message.** The plugin appends `/api/v1/accounts/` to this value, so a URL copied out of the + Chatwoot dashboard's address bar (`https://chat.example.com/app/accounts/2/settings/inboxes/8`) + produced a nonsense request path and 404'd every relay — while the plugin enabled cleanly and looked + healthy until the retries ran out. It now has to be the origin only. + ### Verified - **Reproduced and confirmed against a live OpenWA 0.12.1 host** (Baileys engine) and a self-hosted diff --git a/chatwoot-adapter/backfill.test.ts b/chatwoot-adapter/backfill.test.ts index c755e3e..67a53ca 100644 --- a/chatwoot-adapter/backfill.test.ts +++ b/chatwoot-adapter/backfill.test.ts @@ -60,6 +60,7 @@ function makeDeps( log: () => {}, // Both callbacks are required on InboundDeps; the cast would hide an omitted one. onInboundLost: () => {}, + onRelayError: () => {}, onBackfillExhausted: () => {}, } as unknown as InboundDeps; return { deps, posts, creates, seen }; diff --git a/chatwoot-adapter/echo-loop.test.ts b/chatwoot-adapter/echo-loop.test.ts index cc271b3..933736d 100644 --- a/chatwoot-adapter/echo-loop.test.ts +++ b/chatwoot-adapter/echo-loop.test.ts @@ -75,6 +75,7 @@ async function wire(sessionId = 'sess') { relayGroups: true, relayMedia: true, backfillLimit: 0, backfillAllOnce: false, log: () => {}, // Both callbacks are required on InboundDeps; the cast would hide an omitted one. onInboundLost: () => {}, + onRelayError: () => {}, onBackfillExhausted: () => {}, } as unknown as InboundDeps; @@ -164,6 +165,7 @@ test("one tenant's mirror marker does not suppress another tenant's reply with t client: { postText: async () => { posted.push({ id: 60 }); return { id: 60 }; }, postMedia: async () => ({ id: 60 }) }, // Both callbacks are required on InboundDeps; the cast would hide an omitted one. onInboundLost: () => {}, + onRelayError: () => {}, onBackfillExhausted: () => {}, } as unknown as InboundDeps; await handleSent(inboundA, 'sessA', 'Engine', { ...own, chatId: 'alice@c.us' } as IncomingMessage); @@ -207,6 +209,7 @@ test('an echo webhook processed while the adapter post is still in flight is NOT relayGroups: true, relayMedia: true, backfillLimit: 0, backfillAllOnce: false, log: () => {}, // Both callbacks are required on InboundDeps; the cast would hide an omitted one. onInboundLost: () => {}, + onRelayError: () => {}, onBackfillExhausted: () => {}, } as unknown as InboundDeps; const sent: Array<{ chatId?: string }> = []; diff --git a/chatwoot-adapter/inbound.test.ts b/chatwoot-adapter/inbound.test.ts index a69b66d..16b23f5 100644 --- a/chatwoot-adapter/inbound.test.ts +++ b/chatwoot-adapter/inbound.test.ts @@ -61,6 +61,7 @@ function makeDeps( lock: new KeyedAsyncLock(), client, store: mapping, engine, instanceId: 'inst', relayGroups: true, relayMedia: true, backfillLimit: over.backfillLimit ?? 0, log: over.log ?? (() => {}), onInboundLost: (msgId: string) => void lost.push(msgId), + onRelayError: () => {}, onBackfillExhausted: over.onBackfillExhausted ?? (() => {}), } as unknown as InboundDeps; return { deps: d, counts: () => ({ contacts, convs }), posted, posts, lost }; diff --git a/chatwoot-adapter/inbound.ts b/chatwoot-adapter/inbound.ts index 61d6c36..36514a1 100644 --- a/chatwoot-adapter/inbound.ts +++ b/chatwoot-adapter/inbound.ts @@ -113,6 +113,7 @@ export async function handleInbound( await relayInbound(deps, sessionId, msg); } catch (err) { deps.log('inbound relay failed; queued for retry', err); + deps.onRelayError(err); // Strip an oversized media blob before persisting so a huge value can't be rejected by the storage // layer (which would lose the message — it's already markSeen); the retry then posts a placeholder. const dropped = await deps.store diff --git a/chatwoot-adapter/index.test.ts b/chatwoot-adapter/index.test.ts index 53fc067..604078f 100644 --- a/chatwoot-adapter/index.test.ts +++ b/chatwoot-adapter/index.test.ts @@ -71,6 +71,39 @@ test('relayOwnMessages=false gates the message:sent handler off (no Chatwoot API assert.equal(fetches(), 0); }); +test('onEnable rejects a baseUrl carrying a path (the address-bar copy-paste)', async () => { + // `base()` appends /api/v1/accounts/ to this value, so a URL copied out of the Chatwoot dashboard + // produces .../app/accounts/2/settings/inboxes/8/api/v1/... and 404s every single request — while enable + // succeeds and healthCheck stays green until the retries burn out. + for (const bad of ['https://chat.acme.com/app', 'https://chat.acme.com/app/accounts/2/settings/inboxes/8']) { + const { ctx } = fakeCtx({ ...goodConfig, baseUrl: bad }); + await assert.rejects(new ChatwootAdapter().onEnable(ctx), /baseUrl must be the Chatwoot origin/); + } + // A bare origin, with or without the trailing slash `base()` already strips, stays valid. + for (const ok of ['https://chat.acme.com', 'https://chat.acme.com/', 'https://chat.acme.com:8443']) { + const { ctx } = fakeCtx({ ...goodConfig, baseUrl: ok }); + await new ChatwootAdapter().onEnable(ctx); + } +}); + +test('healthCheck surfaces the last relay error, so the cause is visible without server logs', async () => { + // The whole point of #63: the error exists and is logged, but the log only reaches container stdout. + // The dashboard shows healthCheck's message, so the reason has to ride along with the counters. + const { ctx, cbs } = fakeCtx(goodConfig); + const adapter = new ChatwootAdapter(); + await adapter.onEnable(ctx); + await cbs['message:received']({ + sessionId: 'sess', + source: 'Engine', + data: { id: 'm1', fromMe: false, chatId: '628123@c.us', body: 'hi', type: 'chat', isGroup: false }, + }); + // The hook fires the relay off-thread so a slow Chatwoot never blocks the WA pipeline — let it settle. + for (let i = 0; i < 10; i++) await new Promise(res => setImmediate(res)); + const h = await adapter.healthCheck(); + assert.match(h.message ?? '', /last error: /); + await adapter.onDisable(); +}); + test('healthCheck reports the pending retry backlog (healthy — pending is transient)', async () => { const { ctx, storageMap } = fakeCtx(goodConfig); storageMap.set('retry:sess:m1', { sessionId: 'sess', chatId: 'c@wa', msg: { id: 'm1' }, attempts: 1, enqueuedAt: 1 }); diff --git a/chatwoot-adapter/index.ts b/chatwoot-adapter/index.ts index 162e993..a869299 100644 --- a/chatwoot-adapter/index.ts +++ b/chatwoot-adapter/index.ts @@ -48,6 +48,12 @@ function readConfig(raw: Record): ChatwootFullConfig { if (parsed.protocol !== 'https:' || parsed.username || parsed.password) { throw new Error('chatwoot-adapter: baseUrl must be an https URL without embedded credentials'); } + // A value copied out of the dashboard's address bar (…/app/accounts/2/settings/inboxes/8) parses and + // passes every check above, then has /api/v1/accounts/ appended to it and 404s on every request — + // with the plugin reporting healthy until the retries burn out. Reject it at Save time instead. + if (parsed.pathname !== '/' && parsed.pathname !== '') { + throw new Error('chatwoot-adapter: baseUrl must be the Chatwoot origin only (e.g. https://chat.example.com), with no path'); + } const rawLimit = Number(raw.backfillLimit); return { baseUrl, @@ -66,6 +72,10 @@ export default class ChatwootAdapter implements IPlugin { private retryTimer: ReturnType | null = null; private store: MappingStore | null = null; private deadLetterCount = 0; + // Message of the most recent failed relay, surfaced on healthCheck. The counters say a relay is failing; + // this says WHY (a Chatwoot status + response body, an SSRF refusal, a TLS error), which is otherwise + // only in the host's stdout — the dashboard renders healthCheck's message and no plugin log. + private lastRelayError: string | null = null; // Inbound messages that could neither be relayed nor queued — actual data loss, almost always the host // rejecting a write because the plugin is at its storage quota. Counted separately from dead-lettering: // a dead letter was at least retried MAX_RETRY_ATTEMPTS times, this one never got a single attempt. @@ -108,6 +118,7 @@ export default class ChatwootAdapter implements IPlugin { ctx.logger.error(`inbound message ${msgId} LOST: could not be relayed and could not be queued`, e); }, onBackfillExhausted: this.onBackfillExhausted, + onRelayError: (e: unknown) => void (this.lastRelayError = e instanceof Error ? e.message : String(e)), }); ctx.registerHook( @@ -259,6 +270,8 @@ export default class ChatwootAdapter implements IPlugin { if (this.backfillExhausted.size > 0) { parts.push(`${this.backfillExhausted.size} chat(s) gave up on history import after ${MAX_BACKFILL_ATTEMPTS} attempts`); } + // Appended last and only when something is actually wrong: on a green plugin it would be stale noise. + if (parts.length && this.lastRelayError) parts.push(`last error: ${this.lastRelayError.slice(0, 300)}`); return { healthy: this.deadLetterCount === 0 && this.lostCount === 0 && !saturated, message: parts.join('; ') || undefined, diff --git a/chatwoot-adapter/relay.ts b/chatwoot-adapter/relay.ts index a684dcb..e273506 100644 --- a/chatwoot-adapter/relay.ts +++ b/chatwoot-adapter/relay.ts @@ -21,6 +21,10 @@ export interface InboundDeps { // lost. The only way this failure reaches an operator: the retry queue can't count an entry it never // managed to store, so healthCheck would otherwise report green while dropping messages. onInboundLost: (msgId: string, err: unknown) => void; + // Called on every failed relay, before the message is queued for retry. The error text is the single + // most useful thing an operator can be told (it carries the Chatwoot status and response body), and + // `log` only reaches the host's stdout — healthCheck is the one channel the dashboard renders. + onRelayError: (err: unknown) => void; // Called once when a chat's history import has burned MAX_BACKFILL_ATTEMPTS. Mirrors onInboundLost: // the durable per-chat counter stops the retries, this makes the give-up visible on healthCheck. onBackfillExhausted: (chatId: string) => void; diff --git a/chatwoot-adapter/sent.test.ts b/chatwoot-adapter/sent.test.ts index 70815c9..7924c6b 100644 --- a/chatwoot-adapter/sent.test.ts +++ b/chatwoot-adapter/sent.test.ts @@ -50,6 +50,7 @@ function deps( log: over.log ?? (() => {}), // Both callbacks are required on InboundDeps; the cast would hide an omitted one. onInboundLost: () => {}, + onRelayError: () => {}, onBackfillExhausted: () => {}, } as unknown as InboundDeps; return { deps: d, counts: () => ({ contacts, convs }), posted, seen };