This is an automated email from the ASF dual-hosted git repository.
sk0x50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new b2f470a3f IGNITE-17281 Added support error codes and trace ids to rest
components. Fixes #926
b2f470a3f is described below
commit b2f470a3f60ef0963c6773b02ebda5b34115f16e
Author: Slava Koptilin <slava.kopti...@gmail.com>
AuthorDate: Fri Jul 8 00:45:10 2022 +0300
IGNITE-17281 Added support error codes and trace ids to rest components.
Fixes #926
---
.../apache/ignite/lang/IgniteCheckedException.java | 11 ++++
.../org/apache/ignite/lang/IgniteException.java | 11 ++++
.../IgniteInternalCheckedExceptionHandler.java | 5 +-
.../handler/IgniteInternalExceptionHandler.java | 5 +-
.../ConfigurationControllerBaseTest.java | 8 ++-
.../java/org/apache/ignite/lang/ErrorGroup.java | 13 ++++-
.../lang/IgniteInternalCheckedException.java | 11 ++++
.../ignite/lang/IgniteInternalException.java | 11 ++++
.../exception/handler/IgniteExceptionHandler.java | 18 +++---
.../handler/IgniteExceptionHandlerTest.java | 66 ++++++++++++++++------
10 files changed, 128 insertions(+), 31 deletions(-)
diff --git
a/modules/api/src/main/java/org/apache/ignite/lang/IgniteCheckedException.java
b/modules/api/src/main/java/org/apache/ignite/lang/IgniteCheckedException.java
index 88a8eeb16..80bc7f8a3 100755
---
a/modules/api/src/main/java/org/apache/ignite/lang/IgniteCheckedException.java
+++
b/modules/api/src/main/java/org/apache/ignite/lang/IgniteCheckedException.java
@@ -17,6 +17,7 @@
package org.apache.ignite.lang;
+import static org.apache.ignite.lang.ErrorGroup.ERR_PREFIX;
import static org.apache.ignite.lang.ErrorGroup.errorGroupByCode;
import static org.apache.ignite.lang.ErrorGroup.errorMessage;
import static org.apache.ignite.lang.ErrorGroup.errorMessageFromCause;
@@ -171,6 +172,16 @@ public class IgniteCheckedException extends Exception {
return code;
}
+ /**
+ * Returns a human-readable string represents a full error code.
+ * Returned string has the following format: IGN-XXX-nnn, where XXX is a
group name and nnn is an unique error code within a group.
+ *
+ * @return Full error code in a human-readable format.
+ */
+ public String codeAsString() {
+ return ERR_PREFIX + groupName() + '-' + errorCode();
+ }
+
/**
* Returns error group.
*
diff --git
a/modules/api/src/main/java/org/apache/ignite/lang/IgniteException.java
b/modules/api/src/main/java/org/apache/ignite/lang/IgniteException.java
index 7fe71e413..0c482a684 100644
--- a/modules/api/src/main/java/org/apache/ignite/lang/IgniteException.java
+++ b/modules/api/src/main/java/org/apache/ignite/lang/IgniteException.java
@@ -17,6 +17,7 @@
package org.apache.ignite.lang;
+import static org.apache.ignite.lang.ErrorGroup.ERR_PREFIX;
import static org.apache.ignite.lang.ErrorGroup.errorGroupByCode;
import static org.apache.ignite.lang.ErrorGroup.errorMessage;
import static org.apache.ignite.lang.ErrorGroup.errorMessageFromCause;
@@ -212,6 +213,16 @@ public class IgniteException extends RuntimeException {
return code;
}
+ /**
+ * Returns a human-readable string represents a full error code.
+ * Returned string has the following format: IGN-XXX-nnn, where XXX is a
group name and nnn is an unique error code within a group.
+ *
+ * @return Full error code in a human-readable format.
+ */
+ public String codeAsString() {
+ return ERR_PREFIX + groupName() + '-' + errorCode();
+ }
+
/**
* Returns error group.
*
diff --git
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalCheckedExceptionHandler.java
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalCheckedExceptionHandler.java
index e512686a1..b1da2591a 100644
---
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalCheckedExceptionHandler.java
+++
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalCheckedExceptionHandler.java
@@ -38,7 +38,10 @@ public class IgniteInternalCheckedExceptionHandler
@Override
public HttpResponse<? extends Problem> handle(HttpRequest request,
IgniteInternalCheckedException exception) {
return HttpProblemResponse.from(
-
Problem.fromHttpCode(HttpCode.INTERNAL_ERROR).detail(exception.getMessage())
+ Problem.fromHttpCode(HttpCode.INTERNAL_ERROR)
+ .traceId(exception.traceId())
+ .code(exception.codeAsString())
+ .detail(exception.getMessage())
);
}
}
diff --git
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java
index aa936cbee..e0d037064 100644
---
a/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java
+++
b/modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/rest/exception/handler/IgniteInternalExceptionHandler.java
@@ -37,7 +37,10 @@ public class IgniteInternalExceptionHandler implements
ExceptionHandler<IgniteIn
@Override
public HttpResponse<? extends Problem> handle(HttpRequest request,
IgniteInternalException exception) {
return HttpProblemResponse.from(
-
Problem.fromHttpCode(HttpCode.INTERNAL_ERROR).detail(exception.getMessage())
+ Problem.fromHttpCode(HttpCode.INTERNAL_ERROR)
+ .traceId(exception.traceId())
+ .code(exception.codeAsString())
+ .detail(exception.getMessage())
);
}
}
diff --git
a/modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java
b/modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java
index 04f8d02e1..80d04b581 100644
---
a/modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java
+++
b/modules/configuration/src/test/java/org/apache/ignite/internal/rest/configuration/ConfigurationControllerBaseTest.java
@@ -18,6 +18,8 @@
package org.apache.ignite.internal.rest.configuration;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -104,7 +106,7 @@ public abstract class ConfigurationControllerBaseTest {
var problem = getProblem(thrown);
assertEquals(400, problem.status());
- assertEquals("Configuration value 'no-such-root' has not been found",
problem.detail());
+ assertThat(problem.detail(), containsString("Configuration value
'no-such-root' has not been found"));
}
@Test
@@ -120,7 +122,7 @@ public abstract class ConfigurationControllerBaseTest {
var problem = getProblem(thrown);
assertEquals(400, problem.status());
- assertEquals("'root.subCfg' configuration doesn't have the
'no-such-bar' sub-configuration", problem.detail());
+ assertThat(problem.detail(), containsString("'root.subCfg'
configuration doesn't have the 'no-such-bar' sub-configuration"));
}
@Test
@@ -136,7 +138,7 @@ public abstract class ConfigurationControllerBaseTest {
var problem = getValidationProblem(thrown);
assertEquals(400, problem.status());
- assertEquals("Validation did not pass", problem.detail());
+ assertThat(problem.detail(), containsString("ValidationIssue
[key=root.foo, message=Error word]"));
assertEquals("Error word",
problem.invalidParams().stream().findFirst().get().reason()); // todo: check
name and reason
}
diff --git a/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java
b/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java
index 454f34ca2..f2c288074 100755
--- a/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java
+++ b/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroup.java
@@ -29,6 +29,9 @@ import java.util.UUID;
* Each group can be identified by a name and an integer number that both must
be unique across all error groups.
*/
public class ErrorGroup {
+ /** Additional prefix that is used in a human-readable format of ignite
errors. */
+ public static final String ERR_PREFIX = "IGN-";
+
/** List of all registered error groups. */
private static final Int2ObjectMap<ErrorGroup> registeredGroups = new
Int2ObjectOpenHashMap<>();
@@ -110,8 +113,13 @@ public class ErrorGroup {
* @return New error group.
* @throws IllegalArgumentException If the specified name or group code
already registered.
* or {@code groupCode} is greater than 0xFFFF or less than or equal
to 0.
+ * Also, this exception is thrown if the given {@code groupName} is
{@code null} or empty.
*/
public static synchronized ErrorGroup newGroup(String groupName, int
groupCode) {
+ if (groupName == null || groupName.isEmpty()) {
+ throw new IllegalArgumentException("Group name is null or empty");
+ }
+
String grpName = groupName.toUpperCase(Locale.ENGLISH);
if (registeredGroups.containsKey(groupCode)) {
@@ -187,7 +195,7 @@ public class ErrorGroup {
* @return New error message with predefined prefix.
*/
public static String errorMessage(UUID traceId, String groupName, int
code, String message) {
- return "IGN-" + groupName + '-' + extractErrorCode(code) + " Trace
ID:" + traceId + ((message != null) ? ' ' + message : "");
+ return ERR_PREFIX + groupName + '-' + extractErrorCode(code) + " Trace
ID:" + traceId + ((message != null) ? ' ' + message : "");
}
/**
@@ -214,9 +222,10 @@ public class ErrorGroup {
public static String errorMessageFromCause(UUID traceId, String groupName,
int code, Throwable cause) {
String c = (cause != null && cause.getMessage() != null) ?
cause.getMessage() : null;
- return (c != null && c.startsWith("IGN-")) ? c :
errorMessage(traceId, groupName, code, c);
+ return (c != null && c.startsWith(ERR_PREFIX)) ? c :
errorMessage(traceId, groupName, code, c);
}
+ /** {@inheritDoc} */
@Override
public String toString() {
return "ErrorGroup [name=" + name() + ", code=" + code() + ']';
diff --git
a/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalCheckedException.java
b/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalCheckedException.java
index 2dda664b2..e12ad3a11 100644
---
a/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalCheckedException.java
+++
b/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalCheckedException.java
@@ -17,6 +17,7 @@
package org.apache.ignite.lang;
+import static org.apache.ignite.lang.ErrorGroup.ERR_PREFIX;
import static org.apache.ignite.lang.ErrorGroup.errorGroupByCode;
import static org.apache.ignite.lang.ErrorGroup.errorMessage;
import static org.apache.ignite.lang.ErrorGroup.errorMessageFromCause;
@@ -247,6 +248,16 @@ public class IgniteInternalCheckedException extends
Exception {
return code;
}
+ /**
+ * Returns a human-readable string represents a full error code.
+ * Returned string has the following format: IGN-XXX-nnn, where XXX is a
group name and nnn is an unique error code within a group.
+ *
+ * @return Full error code in a human-readable format.
+ */
+ public String codeAsString() {
+ return ERR_PREFIX + groupName() + '-' + errorCode();
+ }
+
/**
* Returns error group.
*
diff --git
a/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalException.java
b/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalException.java
index 70e5e4e5c..c8e347867 100644
---
a/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalException.java
+++
b/modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalException.java
@@ -17,6 +17,7 @@
package org.apache.ignite.lang;
+import static org.apache.ignite.lang.ErrorGroup.ERR_PREFIX;
import static org.apache.ignite.lang.ErrorGroup.errorGroupByCode;
import static org.apache.ignite.lang.ErrorGroup.errorMessage;
import static org.apache.ignite.lang.ErrorGroup.errorMessageFromCause;
@@ -212,6 +213,16 @@ public class IgniteInternalException extends
RuntimeException {
return code;
}
+ /**
+ * Returns a human-readable string represents a full error code.
+ * Returned string has the following format: IGN-XXX-nnn, where XXX is a
group name and nnn is an unique error code within a group.
+ *
+ * @return Full error code in a human-readable format.
+ */
+ public String codeAsString() {
+ return ERR_PREFIX + groupName() + '-' + errorCode();
+ }
+
/**
* Returns error group.
*
diff --git
a/modules/rest-api/src/main/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandler.java
b/modules/rest-api/src/main/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandler.java
index b1ebb69ff..342e79a6e 100644
---
a/modules/rest-api/src/main/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandler.java
+++
b/modules/rest-api/src/main/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandler.java
@@ -38,27 +38,32 @@ import org.apache.ignite.lang.IgniteException;
@Singleton
@Requires(classes = {IgniteException.class, ExceptionHandler.class})
public class IgniteExceptionHandler implements
ExceptionHandler<IgniteException, HttpResponse<? extends Problem>> {
-
@Override
public HttpResponse<? extends Problem> handle(HttpRequest request,
IgniteException exception) {
- //TODO: set code, traceId when
https://issues.apache.org/jira/browse/IGNITE-17281 is done
-
if (exception.getCause() instanceof IllegalArgumentException) {
return HttpProblemResponse.from(
-
Problem.fromHttpCode(HttpCode.BAD_REQUEST).detail(exception.getCause().getMessage())
+ Problem.fromHttpCode(HttpCode.BAD_REQUEST)
+ .detail(exception.getMessage())
+ .traceId(exception.traceId())
+ .code(exception.codeAsString())
);
}
if (exception.getCause() instanceof ConfigurationValidationException) {
return HttpProblemResponse.from(
ValidationProblem.fromHttpCode(HttpCode.BAD_REQUEST)
- .detail("Validation did not pass")
+ .detail(exception.getMessage())
.invalidParams(mapValidationIssuesToRestFormat((ConfigurationValidationException)
exception.getCause()))
+ .traceId(exception.traceId())
+ .code(exception.codeAsString())
);
}
return HttpProblemResponse.from(
-
Problem.fromHttpCode(HttpCode.INTERNAL_ERROR).detail(exception.getMessage())
+ Problem.fromHttpCode(HttpCode.INTERNAL_ERROR)
+ .detail(exception.getMessage())
+ .traceId(exception.traceId())
+ .code(exception.codeAsString())
);
}
@@ -68,5 +73,4 @@ public class IgniteExceptionHandler implements
ExceptionHandler<IgniteException,
.map(validationIssue -> new
InvalidParam(validationIssue.key(), validationIssue.message()))
.collect(Collectors.toList());
}
-
}
diff --git
a/modules/rest-api/src/test/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandlerTest.java
b/modules/rest-api/src/test/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandlerTest.java
index 7998ae0d0..c666799f5 100644
---
a/modules/rest-api/src/test/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandlerTest.java
+++
b/modules/rest-api/src/test/java/org/apache/ignite/internal/rest/exception/handler/IgniteExceptionHandlerTest.java
@@ -17,12 +17,17 @@
package org.apache.ignite.internal.rest.exception.handler;
+import static org.apache.ignite.lang.ErrorGroup.errorMessage;
+import static org.apache.ignite.lang.ErrorGroup.extractErrorCode;
+import static org.apache.ignite.lang.ErrorGroups.Common.COMMON_ERR_GROUP;
+import static org.apache.ignite.lang.ErrorGroups.Common.UNKNOWN_ERR;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import io.micronaut.http.HttpRequest;
import io.micronaut.http.HttpResponse;
import java.util.List;
+import java.util.UUID;
import java.util.stream.Stream;
import
org.apache.ignite.configuration.validation.ConfigurationValidationException;
import org.apache.ignite.configuration.validation.ValidationIssue;
@@ -30,48 +35,76 @@ import org.apache.ignite.internal.rest.api.InvalidParam;
import org.apache.ignite.internal.rest.api.Problem;
import org.apache.ignite.internal.rest.api.Problem.ProblemBuilder;
import org.apache.ignite.internal.rest.api.ValidationProblem;
+import org.apache.ignite.lang.ErrorGroup;
import org.apache.ignite.lang.IgniteException;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
class IgniteExceptionHandlerTest {
-
private HttpRequest<?> request;
private IgniteExceptionHandler exceptionHandler;
static Stream<Arguments> igniteExceptions() {
+ UUID traceId = UUID.randomUUID();
+ String errorMessage = errorMessage(traceId, UNKNOWN_ERR, null);
+ String humanReadableCode = ErrorGroup.ERR_PREFIX +
COMMON_ERR_GROUP.name() + '-' + extractErrorCode(UNKNOWN_ERR);
+
+ var invalidParams = List.of(
+ new InvalidParam("key1", "Some issue1"),
+ new InvalidParam("key2", "Some issue2"));
+
+ var validationIssues = List.of(
+ new ValidationIssue("key1", "Some issue1"),
+ new ValidationIssue("key2", "Some issue2"));
+
return Stream.of(
Arguments.of(
// given
- new IgniteException("Ooops"),
+ new IgniteException(traceId, UNKNOWN_ERR, "Ooops"),
// expected
- Problem.builder().status(500).title("Internal Server
Error").detail("Ooops")),
+ Problem.builder()
+ .status(500)
+ .title("Internal Server Error")
+ .code(humanReadableCode)
+ .detail(errorMessage + " Ooops")
+ .traceId(traceId)),
Arguments.of(
// given
- new IgniteException(),
+ new IgniteException(traceId, UNKNOWN_ERR),
// expected
- Problem.builder().status(500).title("Internal Server
Error")),
+ Problem.builder()
+ .status(500)
+ .title("Internal Server Error")
+ .code(humanReadableCode)
+ .detail(errorMessage)
+ .traceId(traceId)),
Arguments.of(
// given
- new IgniteException(new
IllegalArgumentException("Illegal value")),
+ new IgniteException(traceId, UNKNOWN_ERR, new
IllegalArgumentException("Illegal value")),
// expected
- Problem.builder().status(400).title("Bad
Request").detail("Illegal value")),
+ Problem.builder()
+ .status(400)
+ .title("Bad Request")
+ .code(humanReadableCode)
+ .traceId(traceId)
+ .detail(errorMessage + " Illegal value")),
Arguments.of(
// given
new IgniteException(
- new ConfigurationValidationException(
- List.of(new ValidationIssue("key1",
"Some issue1"),
- new ValidationIssue("key2",
"Some issue2")))),
+ traceId,
+ UNKNOWN_ERR,
+ new
ConfigurationValidationException(validationIssues)),
// expected
- ValidationProblem.builder().status(400).title("Bad
Request").detail("Validation did not pass")
- .invalidParams(
- List.of(
- new InvalidParam("key1", "Some
issue1"),
- new InvalidParam("key2", "Some
issue2"))))
+ ValidationProblem.builder()
+ .status(400)
+ .title("Bad Request")
+ .detail(errorMessage + ' ' + validationIssues)
+ .code(humanReadableCode)
+ .traceId(traceId)
+ .invalidParams(invalidParams))
);
}
@@ -83,7 +116,6 @@ class IgniteExceptionHandlerTest {
@ParameterizedTest
@MethodSource("igniteExceptions")
- @Disabled("https://issues.apache.org/jira/browse/IGNITE-17305";)
void shouldHandleIgniteException(IgniteException givenIgniteException,
ProblemBuilder<? extends Problem, ?> expectedProblem) {
HttpResponse<? extends Problem> response =
exceptionHandler.handle(request, givenIgniteException);
