Add more failure attributes and specific errors#21
Conversation
SPEC.md
Outdated
| | `UNAUTHENTICATED` | 401 | The client did not supply valid authentication credentials for this request. Clients should not retry this request unless advised otherwise. | | ||
| | `UNAUTHORIZED` | 403 | The caller does not have permission to execute the specified operation. Clients should not retry this request unless advised otherwise. | | ||
| | `NOT_FOUND` | 404 | The requested resource could not be found but may be available in the future. Subsequent requests by the client are permissible but not advised. | | ||
| | `CONFLICT` | 409 | The request could not be made due to a conflict. The may happen when trying to create an operation that has already been started. Clients should not retry this request unless advised otherwise. | |
There was a problem hiding this comment.
Should we also have 408 timeout in this table?
There was a problem hiding this comment.
I was debating but decided against it since I didn't want handlers to return this directly. Let's see if others have opinions here.
There was a problem hiding this comment.
I think we should include it for completeness. I remember some internal confusion about the purpose of that response code.
| "type": "nexus.OperationError", | ||
| }, | ||
| "message": "<Optional error message>", | ||
| "stackTrace": "<Optional stack trace>", |
There was a problem hiding this comment.
How are we supporting stack trace and message encryption?
There was a problem hiding this comment.
See the comment below: "Arbitrary details may be added here as needed."
Quinn-With-Two-Ns
left a comment
Quinn-With-Two-Ns
left a comment
There was a problem hiding this comment.
It would be good to get some eyes @dandavison @cretz or @mjameswh t make sure this will address their issues with the previous approach
|
I haven't really had time to look, but so long as handler errors/exceptions can be like any other failure (with messages and stack traces and causes instead of just causes), works for me. |
bergundy
force-pushed
the
additional-failure-attributes
branch
from
019c09d to
a2d914d
Compare
bergundy
force-pushed
the
additional-failure-attributes
branch
from
a2d914d to
a2addbd
Compare
Implements the changes mentioned in nexus-rpc/api#21. I've added tests in the last commit that should not be merged, they are used to test current main server and client against this version to ensure compatibility. Note that backwards compatibility is only ensured if an error includes only `Cause` or `Message` but not both, but that is okay as the new `Message` field is an addition. I will wait on merging this until the transport refactor is in to avoid merge conflicts in that branch.
4 participants
causeandstackTraceattributes toFailureHandlerErrorOperationErrorCONFLICThandler error typeREQUEST_TIMEOUThandler error typeNexus-Request-RetryableandNexus-Operation-Stateheaders since this information should be present in the response body.