Skip to content

External data endpoints answer 200 true for submissions that were never delivered #6864

Description

@vshvets-bc

Summary

POST /external and POST /external/{policyId}/{blockTag} answer 200 { "body": true } whether or not the submission was delivered to a policy. The reply is produced before delivery is attempted, and in one of the two handlers it is produced even when no policy was found at all. An external submitter — a registry, a device gateway, an MRV sender — has no way to tell a processed submission from a discarded one.

Where

guardian-service/src/policy-engine/policy-engine.service.ts

RECEIVE_EXTERNAL_DATA_CUSTOM:

this.channel.getMessages<any, any>(PolicyEngineEvents.RECEIVE_EXTERNAL_DATA_CUSTOM,
    async (msg: any) => {
        try {
            new GuardiansService().sendPolicyMessage(PolicyEvents.MRV_DATA_CUSTOM, msg.policyId, {
                policyId: msg.policyId,
                data: msg
            });
            return new MessageResponse(true);
        } catch (error) {
            await logger.error(error, ['GUARDIAN_SERVICE'], null);
            return new MessageError(error, error.code);
        }
    });

RECEIVE_EXTERNAL_DATA:

const policy = await DatabaseServer.getPolicyByTag(data?.policyTag);
if (policy) {
    const policyId = policy.id.toString();
    new GuardiansService().sendPolicyMessage(PolicyEvents.MRV_DATA, policyId, { policyId, data: msg });
}
return new MessageResponse(true);

Three distinct ways a 200 means nothing

  1. The result is discarded. sendPolicyMessage is declared public async sendPolicyMessage<T>(subject, policyId, data, awaitInterval = 100000): Promise<T> (guardian-service/src/helpers/guardians.ts:66) — it awaits a reply. Neither call site awaits it, so the outcome is dropped on the floor.

  2. The try/catch cannot catch this call. Because the promise is not awaited, a rejection never enters the catch. It escapes as an unhandled rejection instead, so NO_RESPONDERS (the policy exists but is not running) and any error raised inside the block are both invisible to the caller and to the error handler that looks like it covers them.

  3. RECEIVE_EXTERNAL_DATA returns true on no match. When getPolicyByTag finds nothing the if (policy) body is skipped and the handler falls through to return new MessageResponse(true). Nothing was sent, and the submitter is told it succeeded. RECEIVE_EXTERNAL_DATA_CUSTOM performs no existence check on msg.policyId at all.

The documented contract reinforces the wrong expectation — api-gateway/src/api/service/external.ts annotates both routes with:

@ApiOkResponse({ description: 'Successful operation.', type: Boolean, example: true })

Reproduce

  • POST /external/{a policy id that does not exist}/{any block tag} → 200 true
  • POST /external/{a real policy id that is not currently running}/{block tag} → 200 true, and the data is never processed
  • POST /external with a policyTag matching no policy → 200 true

Impact

External data submission is the integration surface for third parties, and it is exactly the path where silent loss is most expensive: the submitter is a machine that will not notice, and the data usually cannot be reconstructed later. A dropped MRV submission surfaces much later as missing documents, with no signal at the point of failure and nothing in the response to correlate against.

Suggested direction

At minimum, await the send so a delivery failure reaches the existing catch and the caller sees a 5xx instead of true. Beyond that, decide deliberately what the endpoint promises:

  • resolve the policy first and return an explicit not-found when there is no such policy or tag, rather than true
  • distinguish accepted for processing from processed: if the endpoint is meant to be asynchronous, 202 Accepted describes it honestly, and the @ApiOkResponse should say so
  • if the current fire-and-forget behaviour is intended, the unhandled rejection is still worth fixing, since it is a process-level hazard rather than a contract question

Happy to open a PR for whichever shape you prefer — the choice between "await and report" and "202 plus a documented async contract" is a product decision rather than a mechanical one, so I would rather not pick it unilaterally.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions