oxidecomputer/dropshot

How do you set metadata on HttpError?

Closed this issue · 4 comments

adamchalmers commented

Hi! The docs for HttpError say it includes "optionally: additional metadata describing the issue". But I don't see any method to get/set metadata, nor is there a field for it.

The HttpError docs should describe how this feature works and provide an example. Putting structured data into your errors, for clients to consume, is pretty important to a lot of servers :) Happy to write the docs if y'all can point me to how this works.

davepacheco commented

Sorry. It looks like that quote is from the very early days and was probably more aspirational and directional than real. I don't think there's currently a way to do this. It hasn't been a big pain point for us yet (which is not to say it's not important -- I totally get the motivation).

See also #41, #39. These are related, but not the same thing. One of those solutions might help here. Another thought is that instead of returning HttpError, we could allow consumers to return something that impls a trait that allows us to extract a status code and JSON schema (based solely on the type) and an actual response body (based on the value), similar to HttpCodedResponse. We'd have to think about this.

@ahl thoughts?

adamchalmers commented

No worries! In KittyCAD we're building a lot of stuff on Dropshot, so I'd definitely like to make that feature more than aspirational and actually implement it :) I'm happy to open a PR if we can get consensus on a good direction.

The two mutually-exclusive approaches I can see easily are:

  1. Your solution: generalize the HttpResponse trait so it's generic over E, which defaults to E = HttpError (for backwards-compatibility, and as a reasonable default). E is bounded to that trait you describe (get status code, JSON schema, serialize)
  2. Make HttpError accept a metadata field. It could either be serde_json::Value or could be generic over M: Serialize, default to () and get serialized to JSON by Dropshot.
epompeii commented

I third your idea @davepacheco (@adamchalmers 's number 1).
It would be great to also include a headers method in that trait so that customers headers can be added to error messages. A default impl could also be provided that simply returns None or an empty map, etc.

This would solve #801 for me and make it possible to display these more informative error messages in a web UI, satisfying CORS.

davepacheco commented

Closing as a dup of #39.

  • 👍1