feat(deployer): handle errors from corrupted resource data #1208
Conversation
deployer/src/deployment/run.rs
Outdated
| // We collect into a Result so that if the response contains a resource with corrupted data, | ||
| // we terminate iteration and return error. | ||
| // TODO: investigate how the resource data can get corrupted. | ||
| .collect::<Result<Vec<_>>>()? |
There was a problem hiding this comment.
This implementation errors on any corrupted resource, we could also tracing::error! the bad resource errors, and just return the OK ones. 🤔
There was a problem hiding this comment.
I think returning what converted successfully is a good idea. That way the corrupted ones can be recreated and fixed.
There was a problem hiding this comment.
Refactored in 08af728, for deployment the user will see this error in their logs:
ERROR {error="database::shared::postgres resource config should be valid JSON\n\nCaused by:\n expected `:` at line 1 column 13"} shuttle_deployer::deployment::run: failed to parse resource data
For the resource list the tracing error will not be recorded in the deployer sqlite (but maybe we will instrument it with deployment id for new logger?), I can only see it in the deployers docker logs:
2023-09-07T12:17:43.227645Z ERROR request{http.uri=/projects/postgres-rocket-app/services/postgres-rocket-app/resources http.method=GET request.path="/projects/:project_name/services/:service_name/resources" account.name="admin" request.params.service_name="postgres-rocket-app" request.params.project_name="postgres-rocket-app"}:get_service_resources{project_name=postgres-rocket-app service_name=postgres-rocket-app}: shuttle_deployer::handlers: failed to parse resource data error=database::shared::postgres resource config should be valid JSON
Caused by:
expected `:` at line 1 column 13
deployer/src/deployment/run.rs
Outdated
| // We collect into a Result so that if the response contains a resource with corrupted data, | ||
| // we terminate iteration and return error. | ||
| // TODO: investigate how the resource data can get corrupted. | ||
| .collect::<Result<Vec<_>>>()? |
There was a problem hiding this comment.
I think returning what converted successfully is a good idea. That way the corrupted ones can be recreated and fixed.
oddgrd
changed the title
| fn try_from(resource: record_request::Resource) -> Result<Self, Self::Error> { | ||
| let r#type = shuttle_common::resource::Type::from_str(resource.r#type.as_str()) | ||
| .map_err(anyhow::Error::msg) | ||
| .context("resource type should have a valid resource string")?; |
There was a problem hiding this comment.
Proper error handling. This is nice 🤩
oddgrd
deleted the
feature/eng-1215-handle-errors-from-corrupted-resource-data
branch
Description of change
We did not handle errors from corrupted resource data received in the deployer. With this change we will return an internal error with a message about which resource had corrupted data. Figuring out how the data can get corrupted is still a TODO.
How has this been tested? (if applicable)
Tested on local environment by deploying a rocket postgres project, and then manually editing the resource-recorder state and making the config for the postgres resource invalid JSON.
Errors from corrupted resource date:
For deployment the user will see this error in their logs:
For the resource list the tracing error will not be recorded in the deployer sqlite, I can only see it in the deployers docker logs: