-
Notifications
You must be signed in to change notification settings - Fork 2
Improve error model and serialization #44
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,76 @@ | ||
| package io.nexusrpc; | ||
|
|
||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| /** An operation has failed or was canceled. */ | ||
| public class OperationException extends Exception { | ||
| private final OperationState state; | ||
|
|
||
| private OperationException(OperationState state, Throwable cause) { | ||
| super(cause); | ||
| private OperationException(OperationState state, String message, @Nullable Throwable cause) { | ||
| super(message, cause); | ||
| this.state = state; | ||
| } | ||
|
|
||
| /** | ||
| * Create a failed operation exception with a message. | ||
| * | ||
| * @param message The failure message. | ||
| * @return The operation exception. | ||
| */ | ||
| public static OperationException failure(String message) { | ||
| return new OperationException(OperationState.FAILED, message, null); | ||
| } | ||
|
|
||
| /** | ||
| * Create a failed operation exception with a cause. | ||
| * | ||
| * @param cause The cause of the failure. | ||
| * @return The operation exception. | ||
| */ | ||
| public static OperationException failure(Throwable cause) { | ||
| return new OperationException(OperationState.FAILED, cause); | ||
| return new OperationException(OperationState.FAILED, cause.toString(), cause); | ||
| } | ||
|
|
||
| /** | ||
| * Create a failed operation exception with a message and cause. | ||
| * | ||
| * @param message The failure message. | ||
| * @param cause The cause of the failure. | ||
| * @return The operation exception. | ||
| */ | ||
| public static OperationException failure(String message, Throwable cause) { | ||
| return new OperationException(OperationState.FAILED, message, cause); | ||
| } | ||
|
|
||
| /** | ||
| * Create a canceled operation exception with a message. | ||
| * | ||
| * @param message The cancellation message. | ||
| * @return The operation exception. | ||
| */ | ||
| public static OperationException canceled(String message) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noticed we are inconsistent with the naming. I would choose |
||
| return new OperationException(OperationState.CANCELED, message, null); | ||
| } | ||
|
|
||
| /** | ||
| * Create a canceled operation exception with a cause. | ||
| * | ||
| * @param cause The cause of the cancellation. | ||
| * @return The operation exception. | ||
| */ | ||
| public static OperationException canceled(Throwable cause) { | ||
| return new OperationException(OperationState.CANCELED, cause); | ||
| return new OperationException(OperationState.CANCELED, cause.toString(), cause); | ||
| } | ||
|
|
||
| /** | ||
| * Create a canceled operation exception with a message and cause. | ||
| * | ||
| * @param message The cancellation message. | ||
| * @param cause The cause of the cancellation. | ||
| * @return The operation exception. | ||
| */ | ||
| public static OperationException canceled(String message, Throwable cause) { | ||
| return new OperationException(OperationState.CANCELED, message, cause); | ||
| } | ||
|
|
||
| public OperationState getState() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package io.nexusrpc.handler; | ||
|
|
||
| import io.nexusrpc.FailureInfo; | ||
| import java.util.Arrays; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
|
|
@@ -33,36 +34,160 @@ public enum RetryBehavior { | |
| private final String rawErrorType; | ||
| private final ErrorType errorType; | ||
| private final RetryBehavior retryBehavior; | ||
| private final FailureInfo originalFailure; | ||
|
|
||
| public HandlerException(ErrorType errorType, String message) { | ||
| this(errorType, new RuntimeException(message), RetryBehavior.UNSPECIFIED); | ||
| /** | ||
| * Create a handler exception with the given error type and cause message. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param causeMessage The cause message. | ||
| * @deprecated | ||
| */ | ||
| public HandlerException(ErrorType errorType, String causeMessage) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like we should have a non deprecated constructor that will take just error type and message but I understand that this would break compatibility. In Go we managed to get around this by creating a new constructor but in Java I'm not sure how we might achieve that. Do you think we could remove this deprecated method and bring it back at some point? |
||
| this(errorType, new RuntimeException(causeMessage), RetryBehavior.UNSPECIFIED); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with the given error type and cause message. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param causeMessage The cause message. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| * @deprecated | ||
| */ | ||
| public HandlerException(ErrorType errorType, String causeMessage, RetryBehavior retryBehavior) { | ||
| this(errorType, new RuntimeException(causeMessage), retryBehavior); | ||
| } | ||
|
|
||
| public HandlerException(ErrorType errorType, String message, RetryBehavior retryBehavior) { | ||
| this(errorType, new RuntimeException(message), retryBehavior); | ||
| /** | ||
| * Create a handler exception with the given error type, message, and cause. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param message The error message. | ||
| * @param cause The cause of this exception. | ||
| */ | ||
| public HandlerException(ErrorType errorType, String message, @Nullable Throwable cause) { | ||
| this(errorType, message, cause, RetryBehavior.UNSPECIFIED); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with the given error type and cause. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param cause The cause of this exception. | ||
| */ | ||
| public HandlerException(ErrorType errorType, @Nullable Throwable cause) { | ||
| this(errorType, cause, RetryBehavior.UNSPECIFIED); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with the given error type, cause, and retry behavior. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param cause The cause of this exception. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| */ | ||
| public HandlerException( | ||
| ErrorType errorType, @Nullable Throwable cause, RetryBehavior retryBehavior) { | ||
| super(cause == null ? "handler error" : "handler error: " + cause.getMessage(), cause); | ||
| this( | ||
| errorType, | ||
| cause == null ? "handler error" : "handler error: " + cause.getMessage(), | ||
| cause, | ||
| retryBehavior); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with the given error type, message, cause, and retry behavior. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param message The error message. | ||
| * @param cause The cause of this exception. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| */ | ||
| public HandlerException( | ||
| ErrorType errorType, String message, @Nullable Throwable cause, RetryBehavior retryBehavior) { | ||
| this(errorType, message, cause, retryBehavior, null); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with the given error type, message, cause, retry behavior, and | ||
| * original failure. | ||
| * | ||
| * @param errorType The error type. | ||
| * @param message The error message. | ||
| * @param cause The cause of this exception. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| * @param originalFailure The original failure information if available. | ||
| */ | ||
| public HandlerException( | ||
| ErrorType errorType, | ||
| String message, | ||
| @Nullable Throwable cause, | ||
| RetryBehavior retryBehavior, | ||
| FailureInfo originalFailure) { | ||
| super(message, cause); | ||
| this.rawErrorType = errorType.name(); | ||
| this.errorType = errorType; | ||
| this.errorType = | ||
| Arrays.stream(ErrorType.values()).anyMatch((t) -> t.name().equals(rawErrorType)) | ||
| ? ErrorType.valueOf(rawErrorType) | ||
| : ErrorType.UNKNOWN; | ||
| this.retryBehavior = retryBehavior; | ||
| this.originalFailure = originalFailure; | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with a raw error type string, cause, and retry behavior. | ||
| * | ||
| * @param rawErrorType The raw error type string. | ||
| * @param cause The cause of this exception. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| */ | ||
| public HandlerException( | ||
| String rawErrorType, @Nullable Throwable cause, RetryBehavior retryBehavior) { | ||
| super(cause == null ? "handler error" : "handler error: " + cause.getMessage(), cause); | ||
| this( | ||
| rawErrorType, | ||
| cause == null ? "handler error" : "handler error: " + cause.getMessage(), | ||
| cause, | ||
| retryBehavior); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with a raw error type string, message, cause, and retry behavior. | ||
| * | ||
| * @param rawErrorType The raw error type string. | ||
| * @param message The error message. | ||
| * @param cause The cause of this exception. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| */ | ||
| public HandlerException( | ||
| String rawErrorType, String message, @Nullable Throwable cause, RetryBehavior retryBehavior) { | ||
| this(rawErrorType, message, cause, retryBehavior, null); | ||
| } | ||
|
|
||
| /** | ||
| * Create a handler exception with a raw error type string, message, cause, retry behavior, and | ||
| * original failure. | ||
| * | ||
| * @param rawErrorType The raw error type string. | ||
| * @param message The error message. | ||
| * @param cause The cause of this exception. | ||
| * @param retryBehavior The retry behavior for this exception. | ||
| * @param originalFailure The original failure information if available. | ||
| */ | ||
| public HandlerException( | ||
| String rawErrorType, | ||
| String message, | ||
| @Nullable Throwable cause, | ||
| RetryBehavior retryBehavior, | ||
| @Nullable FailureInfo originalFailure) { | ||
| super(message, cause); | ||
| this.rawErrorType = rawErrorType; | ||
| this.errorType = | ||
| Arrays.stream(ErrorType.values()).anyMatch((t) -> t.name().equals(rawErrorType)) | ||
| ? ErrorType.valueOf(rawErrorType) | ||
| : ErrorType.UNKNOWN; | ||
| this.retryBehavior = retryBehavior; | ||
| this.originalFailure = originalFailure; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -86,6 +211,12 @@ public RetryBehavior getRetryBehavior() { | |
| return retryBehavior; | ||
| } | ||
|
|
||
| /** Original FailureInfo if available */ | ||
| @Nullable | ||
| public FailureInfo getOriginalFailure() { | ||
| return originalFailure; | ||
| } | ||
|
|
||
| public boolean isRetryable() { | ||
| if (retryBehavior != RetryBehavior.UNSPECIFIED) { | ||
| return retryBehavior == RetryBehavior.RETRYABLE; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
io.nexusrpcis@NullMarked(seepackage-info.java), sogetStackTrace()currently advertises a non-null return type, butBuilder#setStackTraceacceptsnullandbuild()also leaves it unset by default (covered byFailureInfoTest.builderWithoutStackTrace). This createsFailureInfoinstances whosestackTraceisnulldespite a non-null API contract, so callers that trust the signature can dereference it and hit runtime NPEs.Useful? React with 👍 / 👎.