Bump dropshot from b562f4f to fef3a0e

[dependabot]

Bumps dropshot from b562f4f to fef3a0e.

Commits

• fef3a0e Bump rcgen from 0.9.1 to 0.9.2 (#289)
• f744d95 Bump slog-bunyan from 2.3.0 to 2.4.0 (#290)
• 0018f66 Error responses in OpenAPI output (#286)
• 784399a Introduce structured and unstructured response headers (#283)
• 8e668f8 Update actions/setup-node action to v3 (#287)
• 7bb1091 add debug for better integrations with rust tracing (#278)
• See full diff in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
• @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

[davepacheco]

Nothing major here but a few small suggestions. This is neat! I really like the way progenitor splits out the error cases, of which getting an explicit error HTTP response is only one.

[davepacheco]

Can we keep at least the message from the underlying error so that if we see this in the field we'll know what it was?

[ahl]

can do

[davepacheco]

Discussed offline: I think a 503 here should still be a 503 out the front door.

I understand if we don't want to bother being too pedantic here, but there are some other cases that probably shouldn't go straight out: 401 and 403, for example. It might be safer to have an allow-list of codes that we pass through (maybe: 400, 404, 409, 422, 503) and make everything else a 500?

[ahl]

With regard to the allow list: 1. I like it 2. it seems challenging to do robustly without some additional facilities from dropshot. For example, consider a 404 not found:

In the Error type it's represented like this:

ObjectNotFound { type_name: ResourceType, lookup_type: LookupType },

What we get from the progenitor/dropshot call is (effectively) a dropshot::HttpError which has a message field that was populated like this:

                let message = format!(
                    "not found: {} with {} \"{}\"",
                    t, lookup_field, lookup_value
                );
                HttpError::for_client_error(
                    Some(String::from("ObjectNotFound")),
                    http::StatusCode::NOT_FOUND,
                    message,
                )

So: in this impl From<progenitor::progenitor_client::Error<T>> for Error if we were to see that the status was 404 we would need to parse out the message field into the components for the ObjectNotFound variant. That seems both flaky for services within the nexus repo and completely untenable for external services such as crucible which e.g. populates the message field differently:

    Err(HttpError::for_not_found(
        None,
        format!("region {:?} snapshot {:?} not found", p.id, p.name),
    ))

All of this is to say that I agree that maintaining structured errors across service boundaries seems useful and I think we need some more infrastructure to build that robustly.

[dependabot]

A newer version of dropshot exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.