Skip to content

fix(wwjs): finish applying #762's honesty rule to status posts and acks#773

Merged
rmyndharis merged 2 commits into
mainfrom
fix/wwjs-status-result-honesty
Jul 17, 2026
Merged

fix(wwjs): finish applying #762's honesty rule to status posts and acks#773
rmyndharis merged 2 commits into
mainfrom
fix/wwjs-status-result-honesty

Conversation

@rmyndharis

Copy link
Copy Markdown
Owner

The rule #762 set

Client.sendMessage() can resolve with undefined rather than throw. An absent message cannot distinguish "nothing was sent" from "sent, id unreadable", and claiming success for the first is unrecoverable — so it throws. A present Message proves dispatch, so an id it cannot read there carries the empty sentinel, and $1 is read before falling back to it (#747).

toStatusResult sits directly beneath toMessageResult and the doc-comment stating that rule. It followed none of it:

private toStatusResult(msg: Message | undefined): StatusResult {
  const ts = msg?.timestamp ? new Date(msg.timestamp * 1000) : new Date();
  return { statusId: msg?.id?._serialized ?? '',};
}

Two defects

1. It fabricates a 201. Status posts reach the identical Client.js path, so undefined carries the identical ambiguity. Instead of failing, it returned 201 with an empty statusId and a new Date() invented on the spot — for a status that may never have been published. Exactly what the comment above it forbids.

2. It discards a readable id — and this is the more reachable half. It never tries $1, so on a renamed build a status that posted perfectly well returns statusId: "". deleteStatus takes that id as its revoke handle, so the status can never be taken down.

That half is not theoretical. Unlike an ordinary send, a status post still succeeds on an unpatched build: Injected/Utils.js returns the constructed msg on the status branch before ever reaching the Msg.get that fails elsewhere. So the one path that still works when others 500 is the one that loses its id.

The ack listener

Same missing fallback. #765 was right to drop an unreadable id rather than pass undefined into an UPDATE that TypeORM turns into waMessageId = NULLthat guard stays. But it dropped ids readable as $1, stranding the message at sent forever, including the ack < 0 that is the only signal a send failed outright.

Blast radius — traced, not assumed

  • No new HTTP behavior class. A send whose toMessageResult throws already surfaces as a 500: failSendtoClientFacingError passes a plain Error through, and there is no global exception filter. A status post throws bare because it has nothing to persist (status echoes are deliberately not stored) and no hook to fire. Same outcome, same reason.
  • No Swagger change. fix(wwjs): report sends honestly when the engine returns no usable message #762 did not document a 500 on the send routes; this stays consistent with that.
  • No openapi.json delta — no controller or DTO touched.
  • Baileys deliberately untouched. Its send path (:525, :1578) and status path (:1631) both use the empty sentinel, so it is self-consistent. Only whatsapp-web.js disagreed with itself, and changing Baileys would need its own analysis of what undefined means there.
  • !msg is now impossible past the guard, so the msg.timestamp ? … : new Date() fallback now only covers a present message with no timestamp — where the post is proven and only the clock is unknown. Unchanged on purpose.

Tests

Six new cases; all six fail against the previous adapter (5 failed / 173 passed before the ack case was added). They cover: $1 recovered on a status post, unreadable id → sentinel, all three post methods rejecting on undefined, and $1 recovered on an ack. The existing status specs pinned only happy paths, which is why this shipped green.

Verification

  • eslint 0 errors · prettier clean · npm test — 188 suites, 2444 passing (was 2438; +6)

Refs #747, #762, #765.

with undefined rather than throw, an absent message cannot tell "nothing was
sent" apart from "sent, id unreadable", and claiming success for the first is
unrecoverable — so it throws. A present Message proves dispatch, so an id it
cannot read there carries the empty sentinel, and `$1` is read before falling
back to it (#747).

toStatusResult sits directly under toMessageResult and the doc-comment stating
that rule, and followed none of it. It reached the identical Client.js path yet
returned 201 with an empty statusId and a `new Date()` invented on the spot for
a status that may never have been published. It also never tried `$1`, so on a
renamed build a status that posted fine came back with statusId: "" — and
deleteStatus takes that id as its revoke handle, leaving a live status nothing
could take down. That is the more reachable half: unlike an ordinary send, a
status post still succeeds on an unpatched build, because Utils.js returns the
constructed msg before ever reaching the Msg.get that fails.

The ack listener had the same missing fallback. #765 was right to drop an
unreadable id rather than pass undefined into an UPDATE that TypeORM turns into
`waMessageId = NULL`, and that guard stays — but it dropped ids that were
readable as `$1`, stranding the message at SENT forever, including the `ack < 0`
that is the only signal a send failed outright.

No new HTTP behavior class: a send whose toMessageResult throws already surfaces
as a 500 (failSend rethrows a plain Error through toClientFacingError), and a
status post has nothing to persist and no hook to fire, so it throws bare.
Baileys is deliberately untouched — its send and status paths both use the empty
sentinel, so it is self-consistent; only whatsapp-web.js disagreed with itself.

All six new tests fail against the previous adapter.
Self-review caught two things wrong with the first commit.

The bare Error defeated the point. There is no global exception filter — the
repo documents this in message-not-found.error.spec.ts — so a raw Error reaches
the caller as {"statusCode":500,"message":"Internal server error"} and the
carefully worded reason is discarded. A send survives that because failSend
routes its message into the message:failed hook; a status post has no hook, so
HTTP is its only consumer and the message was going nowhere at all. Throw
InternalServerErrorException instead: same 500, same non-retryable semantics,
reason intact. Not 503 — that invites an automatic retry, and a duplicate status
post is worse than an opaque error.

The doc comment also stated a reason that isn't true. Status posts do NOT hit
the same ambiguity as sends: Injected/Utils.js returns the status model from its
isStatus branch before ever reaching the Msg.get miss that makes a send's empty
result ambiguous. The only path back with no message is Client.js's
`if (!chat) return null` — nothing was posted. That makes the throw MORE
justified than the original comment argued, not less, but the wrong reason had
picked the wrong error type.

The test now asserts the exception type, not just the message text — matching on
text alone would pass for a bare Error and miss exactly this regression.
@rmyndharis
rmyndharis force-pushed the fix/wwjs-status-result-honesty branch from 9d1c1cc to b56d673 Compare July 17, 2026 03:00
@rmyndharis
rmyndharis merged commit 0536840 into main Jul 17, 2026
6 checks passed
@rmyndharis
rmyndharis deleted the fix/wwjs-status-result-honesty branch July 17, 2026 03:13
rmyndharis added a commit that referenced this pull request Jul 17, 2026
…ses (#778)

The comment on the message_reaction handler said Reaction "is the one structure upstream's id normalization doesn't reach". It is not the one — it is an example of a pattern. Normalization covers structure constructors and msg.id; what it misses is any key assigned straight through, and whatsapp-web.js has at least three: Reaction.js:21/:51 (this.id = data.msgKey, this.msgId = data.parentMsgKey), Message.js:300 (this.protocolMessageKey), and PollVote.js:72 (this.parentMsgKey).

State the rule instead of miscounting the instances, so the next person reading it looks for the pattern rather than assuming Reaction is special. The guard itself is unchanged and still correct; PollVote has no handler here, so nothing new is needed for it.

Comment only — no behavior change. Split out of the #754 SDK work (#777) deliberately: it shares no subject with those record types, and main had since moved this file (#773).

Refs #747, #765.
rmyndharis added a commit that referenced this pull request Jul 17, 2026
…#779)

The id-rename sweep taught the send (#762), ack and status (#773) paths to read `$1` when `_serialized` is absent. It missed the busiest path in the product.

buildIncomingMessageBase runs on every arriving message (adapter:517 onMessage) and every message sent from a linked phone (:593 onMessage_create), and read msg.id._serialized unguarded. Worse, RawMessageFields declared the id as `{ _serialized: string }`, so on a renamed build `$1` was not merely unread but unrepresentable — the fallback could not be written without a cast, and the `id: string` the mapper produced was undefined at runtime for every inbound message. No dedup key, no reply target, nothing for an ack to match, on the highest-volume path with no defence in depth. The build-time backport normally makes it moot, which is why it survived the sweep: the one path where a patcher failure hurts most is the one that assumed it never would.

The sentinel needed care. `?? ''` alone would have been a NEW bug: unlike saveOutgoingMessage, which normalizes an empty id to NULL at its chokepoint and documents why (the (sessionId, waMessageId) unique index is not partial, so NULL is exempt while a second `''` collides and the row is lost silently), the inbound persist passed incoming.id straight through. The same normalization is now applied where inbound messages are stored, and only then is `''` safe to mean "received, id unreadable".

Both tests fail against the previous mapper.

Also credits @SkywardLab in the chat-list changelog entry: #748 reported that site here first, independently, and the credit outlasts the PR thread.

Refs #747, #762, #773, #748.
rmyndharis added a commit that referenced this pull request Jul 17, 2026
…ith the code (#789)

Pre-release audit of the whole tree. Three code sites and eleven false sentences.

CODE — the id-rename sweep (#762/#765/#773/#779) missed three MsgKey reads.

message_revoke_everyone read both ids unguarded, and revokedId needs the fallback even on a PATCHED tree: Client.js:680 does `revoked_msg.id = { ...message.protocolMessageKey }`, overwriting the normalized id with a raw spread, and protocolMessageKey is normalized by neither the structure constructor nor the injected serializer. It is the one place a fully patched build still returns a raw key. Undefined there means the UPDATE falls back to the notification's own id, matches no row, and the deleted message's text stays in the database and on the dashboard while WhatsApp shows it deleted.

collectStatuses lost the id deleteStatus takes as its revoke handle. getChannelMessages fabricated the literal string "undefined" via String() over an undefined read, and its type declared the id as `{ _serialized: string } | string`, making the renamed field unreadable without a cast — the same structural defect #779 fixed inbound. The contact id beside collectStatuses is a Wid and is deliberately untouched: #747's rename hit MsgKey only.

DOCS — every sentence verified false against source.

docs/06 told whatsapp-web.js operators — the default engine — that status posting returns 501. Untrue since #714: the adapter implements it and openapi.json declares 201 only. #774 fixed this exact copy in status.controller.ts this release and left docs/06 stale, so the correct sentence is ported verbatim rather than rewritten. The inverse is the one that could surprise a user: docs/06 promised the recipients allow-list restricts who sees a status; whatsapp-web.js ignores it and broadcasts to the account's whole status-privacy audience, silently.

Catalog routes documented success bodies no engine can return; the field name `id` is correct there and only the 201 was false. docs/03 marked Phone Link unavailable on wwjs though adapter:925 calls requestPairingCode. docs/18 + sdk/README described three or four SDKs, tabled three of five, omitted Maven Central, and pinned the gateway at 0.7.3. Matrix summary counts recomputed by executing the matrix (123/19/14, 5 adapter-gap), leaving library-limitations=14 and uncertain=0 which were already correct. The catalog evidence refs now cite symbols: they drifted twice in two releases, and this commit shifts them again.

PLUGINS_ENABLED shipped in .env.example and docker-compose, read by nothing — setting it false left the plugin surface fully on. Removed rather than wired: plugins.module is @global() with 8 non-plugin injectors, so a real opt-out is its own change. Routes stay ADMIN-only.

The new revoke test fails against the previous adapter.

Refs #747, #714, #762, #773, #779.
rmyndharis added a commit that referenced this pull request Jul 17, 2026
Cuts v0.8.19 — 22 commits since v0.8.18.

PATCH, not minor: #777's ⚠️ is audience-scoped to typed SDK consumers and touches zero gateway source — the server's payloads are byte-identical. docs/15 scopes MINOR to changed payload semantics; this is fixes plus one backward-compatible addition (the Go SDK). docs/18's "SDKs are versioned independently of the gateway" stays true.

Closes out the $1 id-rename saga across every path — send, ack, status, live inbound (#779) and revocation (#789), the last of which bites even a fully patched install because Client.js overwrites the normalized id with a raw protocolMessageKey spread. Plus the patcher latch fix (#772), honest status posts (#773), the Message Tester status codes (#771, closing #750), typed SDK record alignment (#777, closing #754), four dashboard fixes (#766-#769), and a docs pass removing eleven verified-false sentences.

Gate on this tree: check:versions OK, openapi:check green (version line only), eslint 0 errors both, builds clean both, 2453 unit + 52 e2e + 109 dashboard tests passing, i18n parity passed. Diff is exactly the six files the v0.8.18 cut touched; both lockfile diffs are version-only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant