Defensively handle errors examining app requests. #922

2024-08-14T17:55:41Z

telackey commented

2024-08-14 17:55:41 +00:00

Related to cerc-io/webapp-deployment-status-api#10

There are two issues in that. One is that the output probably changed recently, whether in the client or server, where no matching record is found by ID (Note this is specific to laconic record get --id <v> and does not seem to apply to the similar command to retrieve a record by name, laconic name resolve <n>).

Rather than returning [] it is now returning [ null ]. This cause us to think there was an application record found, and we attempt to treat the null entry like an Application object. That's fixed by filtering out null responses, which is a good precaution for the deployer, though I think it makes sense to ask whether this new behavior by the client/server is correct. Seems suspicious.

The other issue is that all the defensive checks we had in place to deal with broken/bad AppDeploymentRequests were around the build. This error was coming much earlier, merely when parsing and examining the request to see if it needed to be handled at all.

I have now added similar defensive error handling around that portion of the code.

Related to https://git.vdb.to/cerc-io/webapp-deployment-status-api/issues/10 There are two issues in that. One is that the output probably changed recently, whether in the client or server, where no matching record is found by ID (Note this is specific to `laconic record get --id <v>` and does not seem to apply to the similar command to retrieve a record by name, `laconic name resolve <n>`). Rather than returning `[]` it is now returning `[ null ]`. This cause us to think there *was* an application record found, and we attempt to treat the `null` entry like an Application object. That's fixed by filtering out null responses, which is a good precaution for the deployer, though I think it makes sense to ask whether this new behavior by the client/server is correct. Seems suspicious. The other issue is that all the defensive checks we had in place to deal with broken/bad AppDeploymentRequests were around the _build_. This error was coming much earlier, merely when parsing and examining the request to see if it needed to be handled at all. I have now added similar defensive error handling around that portion of the code.

telackey added 2 commits 2024-08-14 17:55:43 +00:00

Handle errors which checking for a missing app. 27f5c9fc21

Defensively handle errors examining app requests.

Lint Checks / Run linter (pull_request) Successful in 31s

Details

Deploy Test / Run deploy test suite (pull_request) Successful in 5m0s

Details

Webapp Test / Run webapp test suite (pull_request) Successful in 4m35s

Details

K8s Deploy Test / Run deploy test suite on kind/k8s (pull_request) Successful in 8m2s

Details

Smoke Test / Run basic test suite (pull_request) Successful in 4m18s

Details

e04631b4d5

telackey requested review from dboreham 2024-08-14 17:55:52 +00:00

telackey requested review from zramsay 2024-08-14 17:56:03 +00:00

zramsay approved these changes

2024-08-14 18:03:39 +00:00

telackey merged commit 5c275aa622 into main

2024-08-14 18:04:32 +00:00

telackey deleted branch telackey/missing_app

2024-08-14 18:04:32 +00:00

telackey referenced this issue from a commit

2024-08-14 18:04:33 +00:00

Defensively handle errors examining app requests. (#922)

This repo is archived. You cannot comment on pull requests.

2 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: cerc-io/stack-orchestrator#922