feat: improved error responses

[lidel]

This PR:

• removes reliance on HTTP semantics for error codes and switches to a generic HTTP 400 with Error object in body that is returned with mandatory reason string and optional details, as proposed in Suggestion for error code to be switch to a string #57

• Every endpoint now has only two response types: success and error, and success has the name of returned object in its description, making the spec docs easier to read:

PREVIEW: https://ipfs.github.io/pinning-services-api-spec/#specUrl=https://raw.githubusercontent.com/ipfs/pinning-services-api-spec/feat/error-codes/ipfs-pinning-service.yaml

Closes #57 @obo20 @andrew @GregTheGreek @priom @jsign @sanderpick @andrewxhill @ipfs/wg-pinning-services

Please provide feedback (even if its just 👍 / 👎)

[obo20]

This looks good to me!

[andrew]

Only thing that springs to mind here is that sometimes the http infrastructure hosting the pinning service API may kick out a different 4XX or 5XX error code before the application server running the API gets chance to respond, clients should probably make sure they handle a variety of regular http errors along side the errors returned from IPFS itself.

[jacobheun]

I think the change to the error response is good, but I don't think we should be getting rid of the other http status codes. At the very least we should retain the 500 response. Having each of the 400 codes we've already added helps to clarify the category of error that should be expected when implementing/using this api.

[obo20]

@lidel I agree with @jacobheun on this one. I think it would be beneficial to maintain the original 400-500 error codes that we had before.

The only changes I think that are needed is just the actual error object that's returned

[priom]

This is a good base to continue, but also an application will handle a variety of 200, 400 & 500 for specific errors ans responses so client can be very informative about it.

[lidel]

Thank you for feedback!

• I've restored old error codes and added example for each:

  

• I changed 500 to 5XX to set proper expectations: reverse proxies may return all sorts of errors (503, 504 etc)

Q: is generic 400 enough as "catch-all" type of error? Or should we add separate 4XX for "custom errors" not covered by the spec?

[jacobheun]

Q: is generic 400 enough as "catch-all" type of error? Or should we add separate 4XX for "custom errors" not covered by the spec?

I think adding the additional 4XX for other errors would be useful. To me this implies that while we've documented common errors you might see, the API may still return other errors, such as 405 which we haven't covered. Should these also be required to return the same error object format as the other, defined 4xx codes? I'm leaning yes, but am somewhat concerned about adding an early restriction on this for code ranges.

[lidel]

Added 4XX:

As for suggesting specific response format for 4XX, those should be ok, as this type of error will be returned by the pinning service app.

The problematic one may be 5XX, where response can come from a reverse proxy that timeouts while trying to talk to the service app. One can configure proxy to return errors in specific format, but realistically one can't assume 5XX error body will known, so clients usually check only the status code, body would be optional/best-effort.

[andrew]

👍