hashgraph / hashgraph/guardian

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

Open
#6,864 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
146
Forks
186
Avg merge
2d 20h
Merged PRs (30d)
126

Description

### 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`:**

```ts
this.channel.getMessages(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`:**

```ts
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(subject, policyId, data, awaitInterval = 100000): Promise` (`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:

```ts
@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.

Contributor guide

Open the contributing guide

Research direction

Start in guardian-service/src/policy-engine/policy-engine.service.ts and read sendPolicyMessage in guardian-service/src/helpers/guardians.ts:66, then compare the route contracts in api-gateway/src/api/service/external.ts. Trace both handlers for missing policies and delivery failures, and confirm the chosen accepted-versus-processed contract is reflected in responses and API annotations.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.