diff options
8 files changed, 72 insertions, 32 deletions
diff --git a/documentapi/abi-spec.json b/documentapi/abi-spec.json index 20043450501..49bdf32bfb1 100644 --- a/documentapi/abi-spec.json +++ b/documentapi/abi-spec.json @@ -484,12 +484,16 @@ "public" ], "methods": [ + "public void <init>()", "public void <init>(long)", "public void <init>(com.yahoo.documentapi.Result$ResultType, java.lang.Error)", + "public void <init>(com.yahoo.documentapi.Result$ResultType, com.yahoo.messagebus.Error)", "public boolean isSuccess()", "public java.lang.Error getError()", + "public com.yahoo.messagebus.Error error()", "public long getRequestId()", - "public com.yahoo.documentapi.Result$ResultType type()" + "public com.yahoo.documentapi.Result$ResultType type()", + "public static com.yahoo.messagebus.Error toError(com.yahoo.documentapi.Result$ResultType)" ], "fields": [] }, diff --git a/documentapi/src/main/java/com/yahoo/documentapi/Result.java b/documentapi/src/main/java/com/yahoo/documentapi/Result.java index 9509a485654..5f16b237833 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/Result.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/Result.java @@ -1,6 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.documentapi; +import com.yahoo.messagebus.Error; +import com.yahoo.messagebus.ErrorCode; + /** * The <i>synchronous</i> result of submitting an asynchronous operation. * A result is either a success or not. If it is not a success, it will contain an explanation of why. @@ -11,25 +14,43 @@ package com.yahoo.documentapi; public class Result { /** Null if this is a success, set to the error occurring if this is a failure */ - private Error error = null; + private final Error error; /** The id of this operation */ - private long requestId; + private final long requestId; + + private final ResultType type; - private ResultType type = ResultType.SUCCESS; + /** Creates a successful result with requestId zero */ + public Result() { + this(0); + } /** * Creates a successful result * * @param requestId the ID of the request */ public Result(long requestId) { + this.error = null; this.requestId = requestId; + type = ResultType.SUCCESS; } /** * Creates a unsuccessful result * + * @deprecated Will be removed on Vespa 8 due to incorrect java.lang.Error + */ + @Deprecated(forRemoval = true, since="7") + public Result(ResultType type, java.lang.Error error) { + this.type = type; + this.error = new Error(0, error.getMessage()); + this.requestId = 0; + } + /** + * Creates a unsuccessful result + * * @param type the type of failure * @param error the error to encapsulate in this Result * @see com.yahoo.documentapi.Result.ResultType @@ -37,6 +58,7 @@ public class Result { public Result(ResultType type, Error error) { this.type = type; this.error = error; + this.requestId = 0; } /** @@ -54,8 +76,12 @@ public class Result { * If this was a success, this method returns null. * * @return the Error, or null + * @deprecated Will be removed on Vespa 8 */ - public Error getError() { return error; } + @Deprecated(forRemoval = true, since="7") + public java.lang.Error getError() { return new java.lang.Error(error.getMessage()); } + + public Error error() { return error; } /** * Returns the id of this operation. The asynchronous response to this operation @@ -85,5 +111,16 @@ public class Result { @Deprecated(since = "7", forRemoval = true) // TODO: Remove on Vespa 8 — this is a Response outcome, not a Result outcome. CONDITION_NOT_MET_ERROR } + public static Error toError(ResultType result) { + switch (result) { + case TRANSIENT_ERROR: + return new Error(ErrorCode.TRANSIENT_ERROR, ResultType.TRANSIENT_ERROR.name()); + case CONDITION_NOT_MET_ERROR: + return new Error(ErrorCode.TRANSIENT_ERROR, ResultType.CONDITION_NOT_MET_ERROR.name()); + case FATAL_ERROR: + return new Error(ErrorCode.FATAL_ERROR, ResultType.FATAL_ERROR.name()); + } + return new Error(ErrorCode.NONE, "SUCCESS"); + } } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/local/LocalAsyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/local/LocalAsyncSession.java index 102fa73f2ec..591b4c80436 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/local/LocalAsyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/local/LocalAsyncSession.java @@ -45,8 +45,8 @@ public class LocalAsyncSession implements AsyncSession { private final Executor executor = Executors.newCachedThreadPool(); private final AtomicReference<Phaser> phaser; - private AtomicLong requestId = new AtomicLong(0); - private AtomicReference<Result.ResultType> result = new AtomicReference<>(SUCCESS); + private final AtomicLong requestId = new AtomicLong(0); + private final AtomicReference<Result.ResultType> result = new AtomicReference<>(SUCCESS); public LocalAsyncSession(AsyncParameters params, LocalDocumentAccess access) { this.handler = params.getResponseHandler(); @@ -163,7 +163,7 @@ public class LocalAsyncSession implements AsyncSession { private Result send(Function<Long, Response> responses, DocumentOperationParameters parameters) { Result.ResultType resultType = result.get(); if (resultType != SUCCESS) - return new Result(resultType, new Error()); + return new Result(resultType, Result.toError(resultType)); ResponseHandler responseHandler = parameters.responseHandler().orElse(this::addResponse); long req = requestId.incrementAndGet(); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java index 279e04c43b4..8809e05caf3 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusAsyncSession.java @@ -26,6 +26,7 @@ import com.yahoo.documentapi.messagebus.protocol.RemoveDocumentMessage; import com.yahoo.documentapi.messagebus.protocol.RemoveDocumentReply; import com.yahoo.documentapi.messagebus.protocol.UpdateDocumentMessage; import com.yahoo.documentapi.messagebus.protocol.UpdateDocumentReply; +import com.yahoo.messagebus.Error; import com.yahoo.messagebus.ErrorCode; import com.yahoo.messagebus.Message; import com.yahoo.messagebus.MessageBus; @@ -187,7 +188,7 @@ public class MessageBusAsyncSession implements MessageBusSession, AsyncSession { return toResult(reqId, session.send(msg)); } } catch (Exception e) { - return new Result(Result.ResultType.FATAL_ERROR, new Error(e.getMessage(), e)); + return new Result(Result.ResultType.FATAL_ERROR, new Error(ErrorCode.FATAL_ERROR, e.toString())); } } @@ -285,8 +286,7 @@ public class MessageBusAsyncSession implements MessageBusSession, AsyncSession { return new Result(reqId); } return new Result( - messageBusErrorToResultType(mbusResult.getError().getCode()), - new Error(mbusResult.getError().getMessage() + " (" + mbusResult.getError().getCode() + ")")); + messageBusErrorToResultType(mbusResult.getError().getCode()), mbusResult.getError()); } private static Response.Outcome toOutcome(Reply reply) { diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java index 318b518f44e..1b0a6db3d53 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusSyncSession.java @@ -112,7 +112,7 @@ public class MessageBusSyncSession implements MessageBusSession, SyncSession, Re Thread.sleep(100); } if (!result.isSuccess()) { - throw new DocumentAccessException(result.getError().toString()); + throw new DocumentAccessException(result.error().toString()); } return monitor.waitForReply(); } catch (InterruptedException e) { diff --git a/documentapi/src/test/java/com/yahoo/documentapi/local/LocalDocumentApiTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/local/LocalDocumentApiTestCase.java index 3a748ca173a..69fa32986f2 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/local/LocalDocumentApiTestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/local/LocalDocumentApiTestCase.java @@ -111,7 +111,7 @@ public class LocalDocumentApiTestCase extends AbstractDocumentApiTestCase { for (DocumentId id : ids) { Result result = session.get(id); if ( ! result.isSuccess()) - throw new IllegalStateException("Failed requesting document " + id, result.getError().getCause()); + throw new IllegalStateException("Failed requesting document " + id + ": " + result.error().toString()); outstandingRequests.add(result.getRequestId()); } diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java index 9ec25f8b6a6..51707a05401 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java @@ -919,7 +919,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { return false; if (result.type() == Result.ResultType.FATAL_ERROR) - throw new DispatchException(result.getError()); + throw new DispatchException(new Throwable(result.error().toString())); outstanding.incrementAndGet(); return true; @@ -1202,7 +1202,7 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler { return false; if (result.type() == Result.ResultType.FATAL_ERROR) - onError.accept(result.getError().getMessage()); + onError.accept(result.error().getMessage()); else outstanding.incrementAndGet(); diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java index 232d49129d7..1fbde14825b 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java @@ -42,7 +42,6 @@ import com.yahoo.documentapi.VisitorParameters; import com.yahoo.documentapi.VisitorResponse; import com.yahoo.documentapi.VisitorSession; import com.yahoo.documentapi.messagebus.protocol.PutDocumentMessage; -import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.test.MockMetric; import com.yahoo.messagebus.StaticThrottlePolicy; import com.yahoo.messagebus.Trace; @@ -364,7 +363,7 @@ public class DocumentV1ApiTest { assertEquals(expectedUpdate, update); parameters.responseHandler().get().handleResponse(new UpdateResponse(0, false)); assertEquals(parameters().withRoute("content"), parameters); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid?selection=true&cluster=content&timeChunk=10", PUT, "{" + @@ -415,7 +414,7 @@ public class DocumentV1ApiTest { assertEquals(expectedRemove, remove); assertEquals(parameters().withRoute("content"), parameters); parameters.responseHandler().get().handleResponse(new DocumentIdResponse(0, doc2.getId(), "boom", Response.Outcome.ERROR)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid?selection=false&cluster=content", DELETE); assertSameJson("{" + @@ -476,7 +475,7 @@ public class DocumentV1ApiTest { assertEquals(doc1.getId(), id); assertEquals(parameters().withRoute("content").withFieldSet("go"), parameters); parameters.responseHandler().get().handleResponse(new DocumentResponse(0, null)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?cluster=content&fieldSet=go&timeout=123"); assertSameJson("{" + @@ -490,7 +489,7 @@ public class DocumentV1ApiTest { assertEquals(doc1.getId(), id); assertEquals(parameters().withFieldSet("music:[document]"), parameters); parameters.responseHandler().get().handleResponse(new DocumentResponse(0, doc1)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?"); assertSameJson("{" + @@ -507,7 +506,7 @@ public class DocumentV1ApiTest { assertEquals(new DocumentId("id:space:music::one/two/three"), id); assertEquals(parameters().withFieldSet("music:[document]"), parameters); parameters.responseHandler().get().handleResponse(new DocumentResponse(0)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid/one/two/three"); assertSameJson("{" + @@ -528,7 +527,7 @@ public class DocumentV1ApiTest { .addChild("Fast Car") .addChild("Baby Can I Hold You")); parameters.responseHandler().get().handleResponse(new DocumentResponse(0, doc2, trace)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two?condition=test%20it&tracelevel=9", POST, "{" + @@ -565,7 +564,7 @@ public class DocumentV1ApiTest { assertEquals(expectedUpdate, update); assertEquals(parameters(), parameters); parameters.responseHandler().get().handleResponse(new UpdateResponse(0, true)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/group/a/three?create=true&timeout=1e1s", PUT, "{" + @@ -617,7 +616,7 @@ public class DocumentV1ApiTest { // PUT on document which is not found is a 200 access.session.expect((update, parameters) -> { parameters.responseHandler().get().handleResponse(new UpdateResponse(0, false)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/docid/sonny", PUT, "{" + @@ -638,7 +637,7 @@ public class DocumentV1ApiTest { assertEquals(expectedRemove, remove); assertEquals(parameters().withRoute("route"), parameters); parameters.responseHandler().get().handleResponse(new DocumentIdResponse(0, doc2.getId())); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two?route=route&condition=false", DELETE); assertSameJson("{" + @@ -669,7 +668,7 @@ public class DocumentV1ApiTest { access.session.expect((id, parameters) -> { assertEquals(clock.instant().plusSeconds(1000), parameters.deadline().get()); parameters.responseHandler().get().handleResponse(new Response(0, "timeout", Response.Outcome.TIMEOUT)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two?timeout=1ks"); assertSameJson("{" + @@ -682,7 +681,7 @@ public class DocumentV1ApiTest { // INSUFFICIENT_STORAGE is a 507 access.session.expect((id, parameters) -> { parameters.responseHandler().get().handleResponse(new Response(0, "disk full", Response.Outcome.INSUFFICIENT_STORAGE)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", DELETE); assertSameJson("{" + @@ -695,7 +694,7 @@ public class DocumentV1ApiTest { // PRECONDITION_FAILED is a 412 access.session.expect((id, parameters) -> { parameters.responseHandler().get().handleResponse(new Response(0, "no dice", Response.Outcome.CONDITION_FAILED)); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", DELETE); assertSameJson("{" + @@ -722,7 +721,7 @@ public class DocumentV1ApiTest { assertEquals(405, response.getStatus()); // OVERLOAD is a 429 - access.session.expect((id, parameters) -> new Result(Result.ResultType.TRANSIENT_ERROR, new Error("overload"))); + access.session.expect((id, parameters) -> new Result(Result.ResultType.TRANSIENT_ERROR, Result.toError(Result.ResultType.TRANSIENT_ERROR))); var response1 = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", POST, "{\"fields\": {}}"); var response2 = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", POST, "{\"fields\": {}}"); var response3 = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two", POST, "{\"fields\": {}}"); @@ -731,7 +730,7 @@ public class DocumentV1ApiTest { " \"message\": \"Rejecting execution due to overload: 2 requests already enqueued\"" + "}", response3.readAll()); assertEquals(429, response3.getStatus()); - access.session.expect((id, parameters) -> new Result(Result.ResultType.FATAL_ERROR, new Error("error"))); + access.session.expect((id, parameters) -> new Result(Result.ResultType.FATAL_ERROR, Result.toError(Result.ResultType.FATAL_ERROR))); handler.dispatchEnqueued(); assertSameJson("{" + " \"pathId\": \"/document/v1/space/music/number/1/two\"," + @@ -748,7 +747,7 @@ public class DocumentV1ApiTest { AtomicReference<ResponseHandler> handler = new AtomicReference<>(); access.session.expect((id, parameters) -> { handler.set(parameters.responseHandler().get()); - return new Result(Result.ResultType.SUCCESS, null); + return new Result(); }); try { var response4 = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?timeout=1ms"); @@ -808,7 +807,7 @@ public class DocumentV1ApiTest { CountDownLatch setup = new CountDownLatch(queueFill); access.session.expect((id, parameters) -> { setup.countDown(); - return new Result(Result.ResultType.TRANSIENT_ERROR, new Error()); + return new Result(Result.ResultType.TRANSIENT_ERROR, Result.toError(Result.ResultType.TRANSIENT_ERROR)); }); for (int i = 0; i < queueFill; i++) { int j = i; |