Invocation error model overhaul#507
Merged
slinkydeveloper merged 3 commits intorestatedev:mainfrom Jun 21, 2023
Merged
Conversation
slinkydeveloper
commented
Jun 15, 2023
This was referenced Jun 15, 2023
Closed
tillrohrmann
approved these changes
Jun 21, 2023
Contributor
tillrohrmann
left a comment
tillrohrmann
left a comment
There was a problem hiding this comment.
Thanks for creating this PR. The changes look good to me. I really like that we now have our own error codes that can be mapped at the periphery to gRPC error codes.
src/common/src/errors.rs
Outdated
Contributor
There was a problem hiding this comment.
I do like that you've introduced our own error code!
src/service_protocol/service-protocol/dev/restate/service/protocol.proto
Outdated
Show resolved
Hide resolved
…8..3f19659 3f19659 Modify the ErrorMessage to carry more info about the journal_mismatch and protocol_violation cases (#27) 8966ee4 Introduce `ErrorMessage` (#26) 4a993ae Protocol spec (#25) git-subtree-dir: src/service_protocol/service-protocol git-subtree-split: 3f19659f7d7feff86775923daf74ba06d1a362ec
* Introduce InvocationErrorCode and InvocationError to model errors arisen during the execution of an invocation. Fix #421 * Implement parsing and conversion of ErrorMessage, plus propagate change i32 -> u32 of the error type. Fix #494
slinkydeveloper
force-pushed
the
issues/421
branch
from
d440d88 to
3c4c6bf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes at the same time #421 and #494. I tried to develop them independently, but at some point I had to intersect those efforts. Depends on restatedev/service-protocol#27.
Changes:
InvocationErrorCodeandInvocationError, to model all the possible invocation errors, and their variants.InvocationErrorCodeis split betweenUserErrorCode, which are the error codes user visible, andRestateErrorCode, which are specific error codes generated by the Restate infrastructure. The former matches the set of error codes from gRPC, and are used for the "exceptional result values" of invocation results. Given the SDK supports it (e.g. supporting theStatusRuntimeExceptionin the java SDK), the user can create failures himself with a code from this set. The latter are used mostly for reporting specific errors generated by Restate, and guide the user through the resolution of given problems.InvocationErrorCode -> UserErrorCode, as some subsystems of Restate, e.g. the gRPC ingress or theInvokeEntryMessage.failure, can understand only the restricted error space given by gRPC. For example, when killing an invocation, the ingress will see this as error codeAborted, while the tracing will notify the error codeKilledto the user to be more specific about the actual problem.ErrorMessage, notably parsing, conversion toInvocationErrorand propagation when retries are not set (more on this topic to follow in next PRs)ErrorCodeandtonic::Codewith eitherInvocationErrorCodeorUserErrorCode, depending on the context. With this change, it is now more clear the context of possible failure cases.