Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,4 @@ public enum ApplicationErrorCategory {
UNSPECIFIED,
/** Expected application error with little/no severity. */
BENIGN,
;
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
* <li>nonRetryable is set to false
* <li>details are set to null
* <li>stack trace is copied from the original exception
* <li>category is set to ApplicationErrorCategory.APPLICATION_ERROR_CATEGORY_UNSPECIFIED
* <li>category is set to {@link ApplicationErrorCategory#UNSPECIFIED}
* </ul>
*/
public final class ApplicationFailure extends TemporalFailure {
Expand All @@ -61,6 +61,16 @@ public final class ApplicationFailure extends TemporalFailure {
private Duration nextRetryDelay;
private final ApplicationErrorCategory category;

/** Creates a new builder for {@link ApplicationFailure}. */
public static ApplicationFailure.Builder newBuilder() {
return new ApplicationFailure.Builder();
}

/** Creates a new builder for {@link ApplicationFailure} initialized with the provided failure. */
public static ApplicationFailure.Builder newBuilder(ApplicationFailure options) {
return new ApplicationFailure.Builder(options);
}

/**
* New ApplicationFailure with {@link #isNonRetryable()} flag set to false.
*
Expand Down Expand Up @@ -178,45 +188,7 @@ public static ApplicationFailure newNonRetryableFailureWithCause(
ApplicationErrorCategory.UNSPECIFIED);
}

/**
* New ApplicationFailure with a specified category and {@link #isNonRetryable()} flag set to
* false.
*
* <p>Note that this exception still may not be retried by the service if its type is included in
* the doNotRetry property of the correspondent retry policy.
*
* @param message optional error message
* @param type error type
* @param category the category of the application failure.
* @param cause failure cause. Each element of the cause chain will be converted to
* ApplicationFailure for network transmission across network if it doesn't extend {@link
* TemporalFailure}
* @param details optional details about the failure. They are serialized using the same approach
* as arguments and results.
*/
public static ApplicationFailure newFailureWithCategory(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this would be a breaking change but this isn't actually released, for any other readers.

String message,
String type,
ApplicationErrorCategory category,
@Nullable Throwable cause,
Object... details) {
return new ApplicationFailure(
message, type, false, new EncodedValues(details), cause, null, category);
}

static ApplicationFailure newFromValues(
String message,
String type,
boolean nonRetryable,
Values details,
Throwable cause,
Duration nextRetryDelay,
ApplicationErrorCategory category) {
return new ApplicationFailure(
message, type, nonRetryable, details, cause, nextRetryDelay, category);
}

ApplicationFailure(
private ApplicationFailure(
String message,
String type,
boolean nonRetryable,
Expand Down Expand Up @@ -262,7 +234,7 @@ public void setNextRetryDelay(Duration nextRetryDelay) {
this.nextRetryDelay = nextRetryDelay;
}

public ApplicationErrorCategory getApplicationErrorCategory() {
public ApplicationErrorCategory getCategory() {
return category;
}

Expand All @@ -274,4 +246,122 @@ private static String getMessage(String message, String type, boolean nonRetryab
+ ", nonRetryable="
+ nonRetryable;
}

public static final class Builder {
private String message;
private String type;
private Values details;
private boolean nonRetryable;
private Throwable cause;
private Duration nextRetryDelay;
private ApplicationErrorCategory category;

private Builder() {}

private Builder(ApplicationFailure options) {
if (options == null) {
return;
}
this.message = options.getOriginalMessage();
this.type = options.type;
this.details = options.details;
this.nonRetryable = options.nonRetryable;
this.nextRetryDelay = options.nextRetryDelay;
this.category = options.category;
}

/**
* Sets the error type of this failure. This is used by {@link
* io.temporal.common.RetryOptions.Builder#setDoNotRetry(String...)} to determine if the
* exception is non retryable.
*/
public Builder setType(String type) {
this.type = type;
return this;
}

/**
* Set the optional error message.
*
* <p>Default is "".
*/
public Builder setMessage(String message) {
this.message = message;
return this;
}

/**
* Set the optional details of the failure.
*
* <p>Details are serialized using the same approach as arguments and results.
*/
public Builder setDetails(Object... details) {
this.details = new EncodedValues(details);
return this;
}

/**
* Set the optional details of the failure.
*
* <p>Details are serialized using the same approach as arguments and results.
*/
public Builder setDetails(Values details) {
this.details = details;
return this;
}

/**
* Set the non retryable flag on the failure.
*
* <p>It means that this exception is not going to be retried even if it is not included into
* retry policy doNotRetry list.
*
* <p>Default is false.
*/
public Builder setNonRetryable(boolean nonRetryable) {
this.nonRetryable = nonRetryable;
return this;
}

/**
* Set the optional cause of the failure. Each element of the cause chain will be converted to
* {@link ApplicationFailure} for network transmission across network if it doesn't extend
* {@link TemporalFailure}.
*/
public Builder setCause(Throwable cause) {
this.cause = cause;
return this;
}

/**
* Set the optional delay before the next retry attempt. Overrides the normal retry delay.
*
* <p>Default is null.
*/
public Builder setNextRetryDelay(Duration nextRetryDelay) {
this.nextRetryDelay = nextRetryDelay;
return this;
}

/**
* Set the optional category of the failure.
*
* <p>Default is {@link ApplicationErrorCategory#UNSPECIFIED}.
*/
public Builder setCategory(ApplicationErrorCategory category) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change the getter on ApplicationFailure to getCategory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also be fine changing the setter instead, any preference?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to the shorter version

this.category = category;
return this;
}

public ApplicationFailure build() {
return new ApplicationFailure(
message,
type,
nonRetryable,
details == null ? new EncodedValues(null) : details,
cause,
nextRetryDelay,
category);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,18 @@ private RuntimeException failureToExceptionImpl(Failure failure, DataConverter d
ApplicationFailureInfo info = failure.getApplicationFailureInfo();
Optional<Payloads> details =
info.hasDetails() ? Optional.of(info.getDetails()) : Optional.empty();
return ApplicationFailure.newFromValues(
failure.getMessage(),
info.getType(),
info.getNonRetryable(),
new EncodedValues(details, dataConverter),
cause,
info.hasNextRetryDelay()
? ProtobufTimeUtils.toJavaDuration(info.getNextRetryDelay())
: null,
FailureUtils.categoryFromProto(info.getCategory()));
return ApplicationFailure.newBuilder()
.setMessage(failure.getMessage())
.setType(info.getType())
.setNonRetryable(info.getNonRetryable())
.setDetails(new EncodedValues(details, dataConverter))
.setCause(cause)
.setNextRetryDelay(
info.hasNextRetryDelay()
? ProtobufTimeUtils.toJavaDuration(info.getNextRetryDelay())
: null)
.setCategory(FailureUtils.categoryFromProto(info.getCategory()))
.build();
}
case TIMEOUT_FAILURE_INFO:
{
Expand Down Expand Up @@ -148,14 +150,12 @@ private RuntimeException failureToExceptionImpl(Failure failure, DataConverter d
info.hasLastHeartbeatDetails()
? Optional.of(info.getLastHeartbeatDetails())
: Optional.empty();
return ApplicationFailure.newFromValues(
failure.getMessage(),
"ResetWorkflow",
false,
new EncodedValues(details, dataConverter),
cause,
null,
ApplicationErrorCategory.UNSPECIFIED);
return ApplicationFailure.newBuilder()
.setMessage(failure.getMessage())
.setType("ResetWorkflow")
.setDetails(new EncodedValues(details, dataConverter))
.setCause(cause)
.build();
}
case ACTIVITY_FAILURE_INFO:
{
Expand Down Expand Up @@ -211,14 +211,12 @@ private RuntimeException failureToExceptionImpl(Failure failure, DataConverter d
case FAILUREINFO_NOT_SET:
default:
// All unknown types are considered to be retryable ApplicationError.
return ApplicationFailure.newFromValues(
failure.getMessage(),
"",
false,
new EncodedValues(Optional.empty(), dataConverter),
cause,
null,
ApplicationErrorCategory.UNSPECIFIED);
return ApplicationFailure.newBuilder()
.setMessage(failure.getMessage())
.setType("")
.setDetails(new EncodedValues(Optional.empty(), dataConverter))
.setCause(cause)
.build();
}
}

Expand Down Expand Up @@ -265,7 +263,7 @@ private Failure exceptionToFailure(Throwable throwable) {
ApplicationFailureInfo.newBuilder()
.setType(ae.getType())
.setNonRetryable(ae.isNonRetryable())
.setCategory(FailureUtils.categoryToProto(ae.getApplicationErrorCategory()));
.setCategory(FailureUtils.categoryToProto(ae.getCategory()));
Optional<Payloads> details = ((EncodedValues) ae.getDetails()).toPayloads();
if (details.isPresent()) {
info.setDetails(details.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ private FailureUtils() {}

public static boolean isBenignApplicationFailure(@Nullable Throwable t) {
if (t instanceof ApplicationFailure
&& ((ApplicationFailure) t).getApplicationErrorCategory()
== ApplicationErrorCategory.BENIGN) {
&& ((ApplicationFailure) t).getCategory() == ApplicationErrorCategory.BENIGN) {
return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright (C) 2022 Temporal Technologies, Inc. All Rights Reserved.
*
* Copyright (C) 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Modifications copyright (C) 2017 Uber Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this material except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.temporal.failure;

import org.junit.Assert;
import org.junit.Test;

public class ApplicationFailureTest {

@Test
public void applicationFailureCopy() {
ApplicationFailure originalAppFailure =
ApplicationFailure.newBuilder().setType("TestType").setMessage("test message").build();
ApplicationFailure newAppFailure =
ApplicationFailure.newBuilder(originalAppFailure).setNonRetryable(true).build();
Assert.assertEquals(originalAppFailure.getType(), newAppFailure.getType());
Assert.assertEquals(
originalAppFailure.getOriginalMessage(), newAppFailure.getOriginalMessage());
Assert.assertNotEquals(originalAppFailure.isNonRetryable(), newAppFailure.isNonRetryable());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,11 @@ public void execute(boolean isBenign) {
if (!isBenign) {
throw ApplicationFailure.newFailure("Non-benign activity failure", "NonBenignType");
} else {
throw ApplicationFailure.newFailureWithCategory(
"Benign activity failure", "BenignType", ApplicationErrorCategory.BENIGN, null);
throw ApplicationFailure.newBuilder()
.setMessage("Benign activity failure")
.setType("BenignType")
.setCategory(ApplicationErrorCategory.BENIGN)
.build();
}
}
}
Expand Down Expand Up @@ -198,8 +201,7 @@ public void activityFailureMetricBenignApplicationError() {
nonBenignErr.getCause().getCause() instanceof ApplicationFailure);
ApplicationFailure af = (ApplicationFailure) nonBenignErr.getCause().getCause();
assertFalse(
"Failure should not be benign",
af.getApplicationErrorCategory() == ApplicationErrorCategory.BENIGN);
"Failure should not be benign", af.getCategory() == ApplicationErrorCategory.BENIGN);
assertEquals("Non-benign activity failure", af.getOriginalMessage());

reporter.assertCounter(
Expand Down Expand Up @@ -227,9 +229,7 @@ public void activityFailureMetricBenignApplicationError() {
"Inner cause should be ApplicationFailure",
benignErr.getCause().getCause() instanceof ApplicationFailure);
ApplicationFailure af2 = (ApplicationFailure) benignErr.getCause().getCause();
assertTrue(
"Failure should be benign",
af2.getApplicationErrorCategory() == ApplicationErrorCategory.BENIGN);
assertTrue("Failure should be benign", af2.getCategory() == ApplicationErrorCategory.BENIGN);
assertEquals("Benign activity failure", af2.getOriginalMessage());

// Expect metrics to remain unchanged for benign failure
Expand Down Expand Up @@ -271,8 +271,7 @@ public void localActivityFailureMetricBenignApplicationError() {
nonBenignErr.getCause().getCause() instanceof ApplicationFailure);
ApplicationFailure af = (ApplicationFailure) nonBenignErr.getCause().getCause();
assertFalse(
"Failure should not be benign",
af.getApplicationErrorCategory() == ApplicationErrorCategory.BENIGN);
"Failure should not be benign", af.getCategory() == ApplicationErrorCategory.BENIGN);
assertEquals("Non-benign activity failure", af.getOriginalMessage());

// Expect metrics to be incremented for non-benign failure
Expand Down Expand Up @@ -300,9 +299,7 @@ public void localActivityFailureMetricBenignApplicationError() {
"Inner cause should be ApplicationFailure",
benignErr.getCause().getCause() instanceof ApplicationFailure);
ApplicationFailure af2 = (ApplicationFailure) benignErr.getCause().getCause();
assertTrue(
"Failure should be benign",
af2.getApplicationErrorCategory() == ApplicationErrorCategory.BENIGN);
assertTrue("Failure should be benign", af2.getCategory() == ApplicationErrorCategory.BENIGN);
assertEquals("Benign activity failure", af2.getOriginalMessage());

// Expect metrics to remain unchanged for benign failure
Expand Down
Loading
Loading