chanzuckerberg / single-cell-data-portal

The data portal supporting the submission, exploration, and management of projects and datasets to cellxgene.
MIT License
61 stars 12 forks source link

Single 400 schema differs from multiple 400 schema #3242

Closed danieljhegeman closed 2 years ago

danieljhegeman commented 2 years ago

Schema for a single 400 error

Schema for a multiple 400 error

I would think these should be identical... @Bento007 you committed both of these sections. Is there context I am missing on these?

Bento007 commented 2 years ago

Change it to

{ 
"details": str,
"title": str,
"type": str,
"invalid-param": {
 "name": str,
 "reason": str,
 "value": str
 }
}

Not all the fields are required in the response.

Bento007 commented 2 years ago

Trying to follow the spirit of this https://www.rfc-editor.org/rfc/rfc7807 the best we can.

danieljhegeman commented 2 years ago

I'm not clear on whether your answer is intended to apply to one or both of '400' and '400 multiple'.

Presumably '400 multiple' is just an array of the schema you've laid out above?

brianraymor commented 2 years ago

Does this mean that we expect to fill in the template?

This document defines a "problem detail" as a way to carry machine- readable details of errors in a HTTP response to avoid the need to define new error response formats for HTTP APIs.

This RFC is focused on a specific problem/solution.

Or we're simply doing this:

https://www.rfc-editor.org/rfc/rfc7807#section-4.2

The "about:blank" URI [RFC6694], when used as a problem type, indicates that the problem has no additional semantics beyond that of the HTTP status code.

When "about:blank" is used, the title SHOULD be the same as the recommended HTTP status phrase for that code (e.g., "Not Found" for 404, and so on), although it MAY be localized to suit client preferences (expressed with the Accept-Language request header).

Bento007 commented 2 years ago

We will be doing the latter @brianraymor.

Bento007 commented 2 years ago

@danieljhegeman If we can consolidate it into a single error that'd be great.

Bento007 commented 2 years ago

keep in mind that the UI also uses these error messages. We might need to update the frontend to match that new error shape, or reshape the errors for the curator API specifically.

Bento007 commented 2 years ago

This does not seem like a P0. What is the motivation for this issue?

danieljhegeman commented 2 years ago

@Bento007 we have been treating all issues with documentation--and especially error messages--as p0. It is something we want to finalize for user experience, plus it is relatively low effort.