Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Agency error and bulk responses #856

Closed
thekaveman opened this issue Apr 28, 2023 · 11 comments
Closed

Agency error and bulk responses #856

thekaveman opened this issue Apr 28, 2023 · 11 comments
Assignees
Labels
Agency Specific to the Agency API question Further information is requested Schema Implications for JSON Schema or OpenAPI
Milestone

Comments

@thekaveman
Copy link
Collaborator

thekaveman commented Apr 28, 2023

This is related to #852 but different enough that I wanted to open a new issue.

Agency references the general Error Messages and Bulk Responses - these describe JSON structures for what succeeded and what failed for a given request. I think this originated in #796 around this commit: 1f4d197

It seems clear that a successful registration/update in an Agency endpoint should return a Bulk Response. #852 is asking which code to use between 200 vs 201.

I'm not as clear on which codes and in what situations either a singular JSON Error Message or a larger JSON Bulk Response (with failures) should be returned?

500

The Responses table for 500 says:

In this case, the answer may contain a text/plain body with an error message for troubleshooting.

It seems like the (JSON) Error Message or Bulk Response should not be used here, since the content type is text/plain. Is that correct?

Or should the language around 500 be updated to allow JSON? Would it return Error Message for GET and Bulk Response for POST/PUT?

409

The Responses table for 409 says:

POST operations when an object already exists and an update is not possible.

This seems like a case for using a Bulk Response - but only for POST? MDN says 409 is more common with PUT.

I also see from the linked commit/PR above that 409 was previously the code associated with these error responses in the Agency docs.

406, 404, and 401

These codes don't really make sense to use with Error Messages or Bulk Responses, no body content is returned with these.

400

Maybe? But then 400 is not referenced in any of the Agency docs, only in the General Info Responses table.

@thekaveman thekaveman added Agency Specific to the Agency API Schema Implications for JSON Schema or OpenAPI labels Apr 28, 2023
@thekaveman thekaveman added this to the 2.0.0 milestone Apr 28, 2023
@thekaveman thekaveman added the question Further information is requested label Apr 28, 2023
@schnuerle
Copy link
Member

Hi @marie-x and @avatarneil do you have some thoughts on this you can share today?

@marie-x
Copy link
Collaborator

marie-x commented May 2, 2023

I'll schedule time with @avatarneil to discuss this morning (Tue 2 May)

@thekaveman
Copy link
Collaborator Author

thekaveman commented May 4, 2023

Here's what I came up with for errors needing Bulk Responses, I'm implementing in the OpenAPI as a starting point but we can always adjust if needed:

Sorted by endpoint

Endpoint HTTP method Error key Error description HTTP code
/event POST bad_param A validation error occurred 400
/event POST missing_param A required parameter is missing 400
/event POST unregistered This device_id is unregistered 404
/reports POST bad_param A validation error occurred 400
/stops POST already_registered A stop with stop_id is already registered 409
/stops POST bad_param A validation error occurred 400
/stops POST missing_param A required parameter is missing 400
/stops PUT bad_param A validation error occurred 400
/stops PUT missing_param A required parameter is missing 400
/stops PUT unregistered This stop_id is unregistered 404
/telemetry POST bad_param A validation error occurred 400
/telemetry POST missing_param A required parameter is missing 400
/telemetry POST unregistered This device_id is unregistered 404
/trips POST bad_param A validation error occurred 400
/trips POST missing_param A required parameter is missing 400
/trips POST unregistered This device_id is unregistered 404
/vehicles POST already_registered A vehicle with device_id is already registered 409
/vehicles POST bad_param A validation error occurred 400
/vehicles POST missing_param A required parameter is missing 400
/vehicles PUT bad_param A validation error occurred 400
/vehicles PUT unregistered This device_id is unregistered 404

Sorted by Error key

Endpoint HTTP method Error key Error description HTTP code
/stops POST already_registered A stop with stop_id is already registered 409
/vehicles POST already_registered A vehicle with device_id is already registered 409
/event POST bad_param A validation error occurred 400
/reports POST bad_param A validation error occurred 400
/stops POST bad_param A validation error occurred 400
/stops PUT bad_param A validation error occurred 400
/telemetry POST bad_param A validation error occurred 400
/trips POST bad_param A validation error occurred 400
/vehicles POST bad_param A validation error occurred 400
/vehicles PUT bad_param A validation error occurred 400
/event POST missing_param A required parameter is missing 400
/stops POST missing_param A required parameter is missing 400
/stops PUT missing_param A required parameter is missing 400
/telemetry POST missing_param A required parameter is missing 400
/trips POST missing_param A required parameter is missing 400
/vehicles POST missing_param A required parameter is missing 400
/event POST unregistered This device_id is unregistered 404
/stops PUT unregistered This stop_id is unregistered 404
/telemetry POST unregistered This device_id is unregistered 404
/trips POST unregistered This device_id is unregistered 404
/vehicles PUT unregistered This device_id is unregistered 404

@thekaveman
Copy link
Collaborator Author

thekaveman commented May 4, 2023

I think the final question on this is for 500 -- should that really be a text/plain response, or should that be application/json with the body of the single-error response?

{
  "error": "...",
  "error_description": "...",
  "error_details": ["...", "..."]
}

@schnuerle
Copy link
Member

I've made a pass across all APIs to align each endpoint to the responses listed in this comment and in the current OpenAPI and Stoplight docs.

Take a look at the release branch to confirm. Example of how I documented it - see the 'Responses' header.

I did not address the 500 question yet. I could go either way on this so welcome feedback @thekaveman @marie-x @avatarneil If it's a quick easy change to OpenAPI (and we think it's a good idea) let's do it today. For MDS spec it's easy now and just changing one line here.

@marie-x
Copy link
Collaborator

marie-x commented May 8, 2023

application/json I'd say

@thekaveman
Copy link
Collaborator Author

It sounds like would want to update the 500 response across all OpenAPI docs then. I'll work on this this morning and get back to this thread.

@thekaveman
Copy link
Collaborator Author

Fixed the 500 response type/schema across all OpenAPI endpoints in openmobilityfoundation/mds-openapi#37

@thekaveman
Copy link
Collaborator Author

I think this issue can be closed after the one-line update in the spec around 500 response type.

@marie-x
Copy link
Collaborator

marie-x commented May 8, 2023

Nice! Thanks!

@schnuerle
Copy link
Member

schnuerle commented May 8, 2023

Alright updated the language here: 0018992 (and then fixed "a" to "an").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agency Specific to the Agency API question Further information is requested Schema Implications for JSON Schema or OpenAPI
Projects
None yet
Development

No branches or pull requests

3 participants