Record why an outgoing SMS failed, so recoverable failures can be retried (Nepal DoIT gateway)

[binokaryg]

What feature do you want to improve?
The Nepal DoIT SMS outgoing service, api/src/services/nepal-doit-sms.js, added in #10225 / #10230.

When a send fails, the service records a fixed reason phrase derived from the HTTP status rather than the error the gateway actually returned, so every 422 is recorded identically on the message doc. It also treats every 422 as final, although the gateway uses that one status both for account-level conditions that clear on their own and for recipients that can never be delivered to.

It would be better to record what the gateway reported and use it to decide whether a failure is worth retrying, so that a recoverable condition resolves itself once it clears instead of requiring manual recovery.

Describe the improvement you'd like

1. Carry the gateway's actual error into details instead of a constant — for example { code: 422, reason: 'UnprocessableContent', error: <body.errors> }. The object form is already supported downstream (docs exist carrying {"reason": "generic; errorCode=0"}). Bound the stored length and leave message content out.
2. Use that reason to classify 422 rather than treating it as one thing.
3. Consider surfacing an account-level failure more prominently than a log line, since that class of error stops every outgoing message on an instance at once rather than affecting one recipient.
4. Include the message id in the error log line, so an entry can be tied back to a message.

The three cases item 2 needs to separate:

Situation	Appropriate handling
Account-level condition, e.g. no balance remaining	retryable — the message should send once the condition clears
Gateway rejects a specific recipient number	terminal
Recipient never resolved — to holds a literal template string such as patient.phone rather than a number	terminal, and must never be retried

The ordering matters: capturing the reason is a prerequisite for the retry change, not merely a diagnostic nicety. The third case returns the same 422 as the first and can never succeed, so retrying 422 without knowing the reason would loop on those messages indefinitely.

Describe alternatives you've considered

• Make all 422s retryable without recording the reason. The smallest change, but unsafe — it would retry messages whose recipient never resolves, forever.

• Rely on the API logs for the reason. This is effectively today's position. The reason exists only in the pod log, subject to whatever log retention the deployment has, whereas the message doc persists indefinitely. Diagnosing a failed message after log retention has elapsed is not possible at all.

• Leave recovery operational — reset messages to pending by script when someone notices. Workable but manual, and the script has no reliable way to select the right messages, since the docs record no usable reason and the three cases above are indistinguishable.

• Ask the gateway to resend. Not available — the DoIT aggregator does not retry failed messages automatically and offers no resend from its dashboard, unlike CHT gateway.

Additional context
STATUS_MAP holds detail as a constant per HTTP status, which is why a 422 always records UnprocessableContent. It is also where 500 carries a retry flag and 422 does not:

cht-core/api/src/services/nepal-doit-sms.js

Lines 7 to 15 in 2b90655

	const STATUS_MAP = {
	// success - based on API response status field
	1: { success: true, state: 'sent', detail: 'Processed' },
	// HTTP error status codes (from error object in catch block)
	401: { success: false, state: 'failed', detail: 'InvalidCredentials' },
	422: { success: false, state: 'failed', detail: 'UnprocessableContent' },
	500: { success: false, state: 'failed', detail: 'InternalServerError', retry: true },
	};

generateStateChange copies that constant straight into details, and that is all that reaches the message doc:

cht-core/api/src/services/nepal-doit-sms.js

Lines 53 to 66 in 2b90655

	const generateStateChange = (message, res) => {
	if (!res) {
	return;
	}
	const status = getStatus(res);
	if (!status || status.retry) {
	return;
	}
	return {
	messageId: message.id,
	state: status.state,
	details: status.detail
	};
	};

The gateway's own error text is already in scope at the point of failure — err.message carries both the status and the body, for example 422 - {"errors":"Balance not enough"} — and is logged, then dropped two lines later:

cht-core/api/src/services/nepal-doit-sms.js

Lines 116 to 123 in 2b90655

	} catch (err) {
	// Handle HTTP errors (401, 422, 500, etc.)
	logger.error(`SMS API error: ${err.message}`);
	const errorStatus = getStatus(err);
	if (errorStatus) {
	// Known error status - generate appropriate state change
	return generateStateChange(message, err);

There is also a related asymmetry worth settling in the same change. If the gateway signals the same condition as HTTP 200 with an unrecognised status field instead of as a 422, validateSuccessResponse returns false, sendMessage returns undefined, and the message is retried indefinitely — the opposite of the 422 path:

cht-core/api/src/services/nepal-doit-sms.js

Lines 68 to 83 in 2b90655

	const validateSuccessResponse = (result) => {
	if (!result) {
	logger.error(`No response received: %o`, result);
	return false;
	}
	logger.debug(`SMS API Response: %o`, result);
	const validResponse = getStatus(result);
	if (!validResponse) {
	logger.error(`SMS API returned status ${result.status}: %o`, result);
	return false;
	}
	return true;
	};

So the same real-world situation is handled as terminal or as endlessly retryable depending only on how the gateway chooses to signal it.

[dacar1988]

Hi @binokaryg, I'd like to work on this issue if it's still available. Could I take it? If so, I'll share my approach here before opening a PR. Thanks.

[dacar1988]

Thanks @binokaryg! A few quick questions before I start:

1. From the DoIT guide and your logs I have "Balance not enough" (retry) and "Invalid number." (final). Have you seen any other errors in practice? And for a 422 we don't recognise, would you rather treat it as final or retry it? Final is safer against loops, but it could permanently fail something that would have gone through later.

2. For the balance case, is it enough to leave the message pending and log the reason, or do you want the reason saved on the message too while it waits?

3. The success response in your feat(#10225): support for centralized sms gateway of nepal #10230 log has an invalid_number array. Have you seen numbers show up there with status 1? Since the API accepts several numbers at once, I'm not sure if that means the whole send failed or just some numbers.

4. For a 200 with a status we don't recognise, what should happen? Right now it retries forever, and since the sender only picks the 25 oldest pending messages each run, enough of those could block every newer message. Do you have examples of what the gateway sends back in that case?

5. For point 3, how would you like account-level failures to show up? For example a counter in the monitoring API, or something in the admin app? Happy to include it here or do it as a follow-up.

Thanks for the help.