Skip to content

Commit

Permalink
Add specific error code to REST API (#2443)
Browse files Browse the repository at this point in the history
* Add specific error code to REST API

* Add `errorCode` to the `NessieError` object, whose JSON representation
  is set to clients as HTTP error response payload. This is necessary to
  allow clients to distinguish Nessie-specific failure mode that fall
  under the same HTTP status code.

* Note: this will break Nessie error response handling in older java clients.

* Add more specific exception classes to `nessie-model`

* Update java client to unwrap and throw (more) specific exceptions based
  on `NessieError` payload error codes.

* Corresponding Python client changes to follow in a separate commit.

* Fix a `TreeApiImpl.meta()` to throw a NessieIllegalArgumentException
  instead of NessieConflictException when the committer is set by the user.

* Refactor `toHash()` in `TreeApiImpl` to remove unnecessary `Optional`

* Minor code cleanup in `AbstractTestRest`

* Make NessieError @Value.Immutable

Closes #477
  • Loading branch information
dimas-b authored Oct 22, 2021
1 parent b49b1ae commit f7ab9aa
Show file tree
Hide file tree
Showing 24 changed files with 568 additions and 291 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@
import com.fasterxml.jackson.databind.ObjectReader;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;
import org.projectnessie.client.http.ResponseContext;
import org.projectnessie.client.http.Status;
import org.projectnessie.error.BaseNessieClientServerException;
import org.projectnessie.error.ErrorCode;
import org.projectnessie.error.ImmutableNessieError;
import org.projectnessie.error.NessieConflictException;
import org.projectnessie.error.NessieError;
import org.projectnessie.error.NessieNotFoundException;
Expand Down Expand Up @@ -52,6 +56,11 @@ public static void checkResponse(ResponseContext con, ObjectMapper mapper) throw
error = decodeErrorObject(status, is, mapper.reader());
}

Optional<BaseNessieClientServerException> modelException = ErrorCode.asException(error);
if (modelException.isPresent()) {
throw modelException.get();
}

switch (status) {
case BAD_REQUEST:
throw new NessieBadRequestException(error);
Expand All @@ -60,8 +69,12 @@ public static void checkResponse(ResponseContext con, ObjectMapper mapper) throw
case FORBIDDEN:
throw new NessieForbiddenException(error);
case NOT_FOUND:
// Report a generic NessieNotFoundException if a sub-class could not be determined from the
// NessieError object
throw new NessieNotFoundException(error);
case CONFLICT:
// Report a generic NessieConflictException if a sub-class could not be determined from the
// NessieError object
throw new NessieConflictException(error);
case TOO_MANY_REQUESTS:
throw new NessieBackendThrottledException(error);
Expand All @@ -77,11 +90,14 @@ private static NessieError decodeErrorObject(
NessieError error;
if (inputStream == null) {
error =
new NessieError(
status.getCode(),
status.getReason(),
"Could not parse error object in response.",
new RuntimeException("Could not parse error object in response."));
ImmutableNessieError.builder()
.errorCode(ErrorCode.UNKNOWN)
.status(status.getCode())
.reason(status.getReason())
.message("Could not parse error object in response.")
.clientProcessingException(
new RuntimeException("Could not parse error object in response."))
.build();
} else {
try {
JsonNode errorData = reader.readTree(inputStream);
Expand All @@ -92,10 +108,20 @@ private static NessieError decodeErrorObject(
// produced by Quarkus and contains the server-side logged error ID. Report the raw JSON
// text to the caller for trouble-shooting.
error =
new NessieError(errorData.toString(), status.getCode(), status.getReason(), null, e);
ImmutableNessieError.builder()
.message(errorData.toString())
.status(status.getCode())
.reason(status.getReason())
.clientProcessingException(e)
.build();
}
} catch (IOException e) {
error = new NessieError(status.getCode(), status.getReason(), null, e);
error =
ImmutableNessieError.builder()
.status(status.getCode())
.reason(status.getReason())
.clientProcessingException(e)
.build();
}
}
return error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;
import org.projectnessie.error.NessieNotFoundException;
import org.projectnessie.error.NessieReferenceNotFoundException;
import org.projectnessie.model.PaginatedResponse;

class TestResultStreamPaginator {
Expand All @@ -38,10 +38,10 @@ void testNotFoundException() {
new ResultStreamPaginator<>(
MockPaginatedResponse::getElements,
(ref, pageSize, token) -> {
throw new NessieNotFoundException("Ref not found");
throw new NessieReferenceNotFoundException("Ref not found");
});
assertThatThrownBy(() -> paginator.generateStream("ref", OptionalInt.empty()))
.isInstanceOf(NessieNotFoundException.class)
.isInstanceOf(NessieReferenceNotFoundException.class)
.hasMessage("Ref not found");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@
import org.projectnessie.client.http.ResponseContext;
import org.projectnessie.client.http.Status;
import org.projectnessie.error.BaseNessieClientServerException;
import org.projectnessie.error.ErrorCode;
import org.projectnessie.error.ImmutableNessieError;
import org.projectnessie.error.NessieConflictException;
import org.projectnessie.error.NessieContentsNotFoundException;
import org.projectnessie.error.NessieError;
import org.projectnessie.error.NessieNotFoundException;
import org.projectnessie.error.NessieReferenceNotFoundException;
import software.amazon.awssdk.utils.StringInputStream;

public class TestResponseFilter {
Expand All @@ -43,9 +47,16 @@ public class TestResponseFilter {

@ParameterizedTest
@MethodSource("provider")
void testResponseFilter(Status responseCode, Class<? extends Exception> clazz) {
final NessieError error =
new NessieError(responseCode.getCode(), responseCode.getReason(), "xxx", null);
void testResponseFilter(
Status responseCode, ErrorCode errorCode, Class<? extends Exception> clazz) {
NessieError error =
ImmutableNessieError.builder()
.message("test-error")
.status(responseCode.getCode())
.errorCode(errorCode)
.reason(responseCode.getReason())
.serverStackTrace("xxx")
.build();
try {
ResponseCheckFilter.checkResponse(new TestResponseContext(responseCode, error), MAPPER);
} catch (Exception e) {
Expand All @@ -63,7 +74,8 @@ void testResponseFilter(Status responseCode, Class<? extends Exception> clazz) {

@Test
void testBadReturn() {
final NessieError error = new NessieError("unknown", 415, "xxx", null);
NessieError error =
ImmutableNessieError.builder().message("unknown").status(415).reason("xxx").build();
assertThatThrownBy(
() ->
ResponseCheckFilter.checkResponse(
Expand Down Expand Up @@ -137,11 +149,13 @@ public InputStream getErrorStream() {
@Test
void testBadReturnBadError() {
NessieError defaultError =
new NessieError(
Status.UNAUTHORIZED.getCode(),
Status.UNAUTHORIZED.getReason(),
"Could not parse error object in response.",
new RuntimeException("Could not parse error object in response."));
ImmutableNessieError.builder()
.status(Status.UNAUTHORIZED.getCode())
.reason(Status.UNAUTHORIZED.getReason())
.message("Could not parse error object in response.")
.clientProcessingException(
new RuntimeException("Could not parse error object in response."))
.build();
assertThatThrownBy(
() ->
ResponseCheckFilter.checkResponse(
Expand All @@ -159,12 +173,20 @@ void testGood() {

private static Stream<Arguments> provider() {
return Stream.of(
Arguments.of(Status.BAD_REQUEST, NessieBadRequestException.class),
Arguments.of(Status.UNAUTHORIZED, NessieNotAuthorizedException.class),
Arguments.of(Status.FORBIDDEN, NessieForbiddenException.class),
Arguments.of(Status.NOT_FOUND, NessieNotFoundException.class),
Arguments.of(Status.CONFLICT, NessieConflictException.class),
Arguments.of(Status.INTERNAL_SERVER_ERROR, NessieInternalServerException.class));
Arguments.of(Status.BAD_REQUEST, ErrorCode.UNKNOWN, NessieBadRequestException.class),
Arguments.of(Status.UNAUTHORIZED, ErrorCode.UNKNOWN, NessieNotAuthorizedException.class),
Arguments.of(Status.FORBIDDEN, ErrorCode.UNKNOWN, NessieForbiddenException.class),
Arguments.of(
Status.NOT_FOUND, ErrorCode.CONTENTS_NOT_FOUND, NessieContentsNotFoundException.class),
Arguments.of(
Status.NOT_FOUND,
ErrorCode.REFERENCE_NOT_FOUND,
NessieReferenceNotFoundException.class),
Arguments.of(Status.NOT_FOUND, ErrorCode.UNKNOWN, NessieNotFoundException.class),
Arguments.of(Status.CONFLICT, ErrorCode.REFERENCE_CONFLICT, NessieConflictException.class),
Arguments.of(Status.CONFLICT, ErrorCode.UNKNOWN, NessieConflictException.class),
Arguments.of(
Status.INTERNAL_SERVER_ERROR, ErrorCode.UNKNOWN, NessieInternalServerException.class));
}

private static class TestResponseContext implements ResponseContext {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import org.projectnessie.client.api.NessieApiV1;
import org.projectnessie.client.http.HttpClientBuilder;
import org.projectnessie.client.tests.AbstractSparkTest;
import org.projectnessie.error.NessieConflictException;
import org.projectnessie.error.BaseNessieClientServerException;
import org.projectnessie.error.NessieNotFoundException;
import org.projectnessie.model.Branch;
import org.projectnessie.model.Contents;
Expand Down Expand Up @@ -64,7 +64,7 @@ void createClient() {
}

@AfterEach
void closeClient() throws NessieNotFoundException, NessieConflictException {
void closeClient() throws BaseNessieClientServerException {
Reference ref = null;
try {
ref = api.getReference().refName("test").get();
Expand All @@ -82,7 +82,7 @@ void closeClient() throws NessieNotFoundException, NessieConflictException {
}

@Test
void testBranches() throws NessieNotFoundException, NessieConflictException {
void testBranches() throws BaseNessieClientServerException {
Dataset<Row> targetTable =
createKVDataSet(
Arrays.asList(tuple2(1, 10), tuple2(2, 20), tuple2(3, 30), tuple2(4, 40)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public int getStatus() {
return status;
}

public ErrorCode getErrorCode() {
return ErrorCode.UNKNOWN;
}

public String getReason() {
return reason;
}
Expand Down
46 changes: 46 additions & 0 deletions model/src/main/java/org/projectnessie/error/ErrorCode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (C) 2020 Dremio
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file 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 org.projectnessie.error;

import java.util.Optional;
import java.util.function.Function;

/**
* Defines Nessie error codes that are more fine-grained than HTTP status codes and maps them to
* exception classes.
*
* <p>The enum names also designate error code in the JSON representation of {@link NessieError}.
*/
public enum ErrorCode {
UNKNOWN(null),
REFERENCE_NOT_FOUND(NessieReferenceNotFoundException::new),
REFERENCE_ALREADY_EXISTS(NessieReferenceAlreadyExistsException::new),
CONTENTS_NOT_FOUND(NessieContentsNotFoundException::new),
REFERENCE_CONFLICT(NessieReferenceConflictException::new),
;

private final Function<NessieError, ? extends BaseNessieClientServerException> exceptionBuilder;

<T extends BaseNessieClientServerException> ErrorCode(Function<NessieError, T> exceptionBuilder) {
this.exceptionBuilder = exceptionBuilder;
}

public static Optional<BaseNessieClientServerException> asException(NessieError error) {
return Optional.ofNullable(error.getErrorCode())
.flatMap(e -> Optional.ofNullable(e.exceptionBuilder))
.map(b -> b.apply(error));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@
*/
package org.projectnessie.error;

/**
* Base class for all exceptions that are represented by the HTTP {@code Conflict} status code
* (409).
*
* <p>This exception should not be instantiated directly on the server-side. It may be instantiated
* and thrown on the client side to represent cases when the server responded with the HTTP {@code
* Conflict} status code, but no fine-grained error information was available.
*/
public class NessieConflictException extends BaseNessieClientServerException {

public NessieConflictException(String message, Throwable cause) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (C) 2020 Dremio
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file 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 org.projectnessie.error;

import org.projectnessie.model.ContentsKey;

/** This exception is thrown when the requested contents object is not present in the store. */
public class NessieContentsNotFoundException extends NessieNotFoundException {

public NessieContentsNotFoundException(ContentsKey key, String ref) {
super(String.format("Could not find contents for key '%s' in reference '%s'.", key, ref));
}

public NessieContentsNotFoundException(NessieError error) {
super(error);
}

@Override
public ErrorCode getErrorCode() {
return ErrorCode.CONTENTS_NOT_FOUND;
}
}
Loading

0 comments on commit f7ab9aa

Please sign in to comment.