[dacar1988]

Description

Problem. The Nepal DoIT SMS service (api/src/services/nepal-doit-sms.js) stored a fixed label per HTTP status, so every 422 was recorded as UnprocessableContent and treated as final. A Balance not enough outage therefore failed messages permanently, and nothing on the doc said why. Two more cases were wrong: a 200 with status: 1 and a non-empty invalid_number was marked sent, and a 200 JSON reply with an unrecognised status was retried forever (and enough of those block the 25-message queue).

What changed (one file; the HTTP request itself is unchanged). The rules follow @binokaryg's answers on the issue (a month of production logs + live API probes):

Gateway outcome	Result	Stored state_details
422 with a string errors matching /balance/i	retried: no change, logged with the message id	none (no doc write per retry)
any other 422 (incl. errors.mobile validation, i.e. unresolved recipients)	failed	{ code: 422, reason: 'UnprocessableContent', error }
401 (rotated/expired key: account-level, like balance)	retried: no change, logged with the message id	none
500, 504, other HTTP errors, network/JSON-parse errors	retried (as before)	none
200, status: 1 (or '1'), empty or missing invalid_number	sent (a warning is logged when it's missing or not an array)	'Processed' (as before)
200, status: 1 (or '1'), non-empty invalid_number	failed	{ code: 200, reason: 'InvalidNumber', error: '["98…"]' }
200, a JSON object with any other status (unknown or missing)	failed	{ code: 200, reason: 'InvalidResponse', error: <body excerpt> }
200, a body that is not a JSON object (HTML page, empty or text body)	retried: no change, logged at error with the message id and a body excerpt	none

error is the gateway's own errors/message text (or a body excerpt), cut to 255 UTF-16 units and made well-formed with toWellFormed() (Node ≥ 22.15 per engines). A split surrogate pair would make CouchDB reject the whole batch write (invalid UTF-8 JSON), losing every state change in it and resending those messages each minute; we reproduced that and guard it with tests. Error log lines now include the message id; account-level retries (balance, 401) are logged once, at warn, instead of an error line plus a warning.

Choices worth flagging

• Success needs status exactly 1 or '1' (master matched both; true or ' 1 ' are not accepted). Per @binokaryg's follow-up, a missing or non-array invalid_number still counts as success (with a warning), so a gateway format change can't fail every message; only a non-empty array fails. 401 is now retried (with a warning) instead of failed.
• A 200 whose body is not a JSON object (a proxy, maintenance or nginx page; couch-request returns those as text) is not a gateway verdict, so it is retried like the 504 page, as @binokaryg asked in review.
• An empty or malformed application/json body still throws inside couch-request before we see it, so it stays on the retry path, like network errors.
• The Invalid number. test data comes from the DoIT guide; per the maintainer it never occurs in production. The production-backed cases are errors.mobile and the invalid_number response.
• Not in this PR (agreed on the issue): a cap on how old a retried message may be, and surfacing account-level failures in monitoring (messaging.outgoing.gateway) / GET /api/balance. Messages already failed by the old code are not retried automatically.

How it was tested

• Unit: api/tests/mocha/services/nepal-doit-sms.spec.js (43 tests), e.g. retries a 422 balance error instead of failing the message, matches any balance error case-insensitively, fails a number the gateway lists as invalid, logging the message id (the maintainer's live response), fails any other status in a successful response, without mapping it to an HTTP status, keeps recorded response bodies bounded and well-formed, stores a well-formed error when the cut splits a character, whatever the error source, still retries a 504 gateway page. On master's service 36 of them fail; each new rule was also checked against deliberately broken variants. npm run lint and all unit suites (webapp, admin, shared-libs, api, sentinel, couchdb) pass.
• End-to-end on a local CHT (CouchDB + this API) against a fake DoIT gateway returning real DoIT-style bodies, 19 scenarios, master vs this branch, over an outage and a top-up. Before: the rejected number is sent, balance messages stay failed after the top-up, unknown/empty 200s are retried forever. After: balance messages are sent after the top-up, everything terminal is attempted once, a successful sibling is sent exactly once, no CouchDB errors.
• From the app UI: messages to a contact with a working number are sent exactly once (happy path); a message sent to a contact whose number the (fake) gateway rejects with the maintainer's invalid_number response ends up failed (InvalidNumber) in the conversation view; on master it shows sent.
• Admin app, Outgoing messages → Due, before / after:

before

after admin queue 18 sms

user message failed

Note: the screenshots predate the last two commits; since then a 401 and a non-JSON 200 page are retried (the message stays pending) instead of failed.

Closes #11438

Code review checklist

• Readable: Concise, well named, follows the style guide
• Tested: Unit and/or e2e where appropriate
• Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.
• AI disclosure: Please disclose use of AI per the guidelines.

AI Disclosure: Research, plan, tests and implementation were produced with Claude Code (Opus), with Codex (GPT) implementing parts and reviewing the research, plan, tests and diff, under my direction; I reviewed the plan, the diff and ran the manual checks myself.

🤖 Generated with Claude Code

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

[binokaryg]

Thanks @dacar1988 for taking this on.

The five rules from the issue are all implemented and tested, the two follow-ups from the 15th are in, and the spec covers the production shapes well. One retry decision goes the wrong way for a case the gateway has already produced; that is the only thing I would hold on. The rest are suggestions, all inline.

[dacar1988]

Thanks @binokaryg, all of it is in the latest commit:

• A 200 whose body is not a JSON object (HTML page, empty or text) is now retried with an error log (message id plus a bounded excerpt) instead of failing; a JSON object with another status stays terminal.
• Account-level retries (401, balance) log once at warn, with no extra error line; a 401 without JSON text falls back to the body excerpt instead of undefined.
• status 1 or '1' is accepted, as on master. I didn't use Number(status) === 1, because it would also accept true or ' 1 '.
• InvalidNumber and InvalidResponse now carry code: 200.
• STATUS_MAP and getStatus are gone; it branches on err.statusCode.
• Tests updated as you described: two flipped, one removed, plus new cases for the non-object bodies, the log content and code: 200.