From 95b2427bfdea6b2fac595c61f50a4cb20170e22a Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:23:18 -0300 Subject: [PATCH 1/7] feat(client): add isDefinite() to client errors Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../conductor/client/http/ApiException.java | 44 ++++++++++++++ .../ConductorClientExceptionTest.java | 57 +++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java b/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java index 5ad12e1af..a14e13760 100644 --- a/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java +++ b/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java @@ -19,6 +19,7 @@ import com.netflix.conductor.common.validation.ValidationError; +import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Data; import lombok.Setter; @@ -35,10 +36,16 @@ static boolean initPreferErrOverResponse() { private final static boolean PREFER_ERR_OVER_RESPONSE = initPreferErrOverResponse(); + private static final int HTTP_REQUEST_TIMEOUT = 408; + private static final int HTTP_CONFLICT = 409; + private static final int HTTP_LOCKED = 423; + private static final int HTTP_TOO_MANY_REQUESTS = 429; + private int status; private String instance; private String code; @Setter private boolean retryable; + @JsonIgnore @Setter private boolean definite; private List validationErrors; //List of validation errors. Available when the status code is 400 private Map> responseHeaders; private String responseBody; @@ -94,6 +101,43 @@ public boolean isClientError() { return getStatus() > 399 && getStatus() < 499; } + /** + * Whether this error proves the request had no effect. + * + *

{@code true} means the server never applied the request, so retrying it is safe. + * + *

{@code false} means the outcome is unknown: the request may or may not have been applied. + * It does not mean the request succeeded, and it does not mean retrying is unsafe — only that + * a retry may duplicate the work. {@code false} is the default, because most transport + * failures prove nothing. + * + *

A dropped connection is always indeterminate, including {@code ConnectException}. OkHttp + * may retry a request on a fresh route after a pooled connection fails mid-send, so "failed to + * connect" can follow a request the server already received. + * + *

This is not {@link #isRetryable()}. That one says whether trying again is worth it; this + * one says whether trying again can duplicate work. They are independent and often opposite: a + * 503 is retryable and indeterminate at the same time. + */ + public boolean isDefinite() { + return definite; + } + + /** + * Whether an HTTP status proves the server rejected the request without applying it. + * + *

Most 4xx codes qualify. Four do not: Conductor can return 408, 409, 423 and 429 after it + * has already written, so a caller that retried them could duplicate the work. + */ + public static boolean definiteFor(int status) { + return status >= 400 + && status < 500 + && status != HTTP_REQUEST_TIMEOUT + && status != HTTP_CONFLICT + && status != HTTP_LOCKED + && status != HTTP_TOO_MANY_REQUESTS; + } + /** * @return HTTP status code */ diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java index e5ea00b8f..d05d2a225 100644 --- a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java +++ b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java @@ -15,8 +15,14 @@ import java.util.List; import java.util.Map; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import io.orkes.conductor.client.http.ApiException; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; + import static org.junit.jupiter.api.Assertions.*; class ConductorClientExceptionTest { @@ -132,4 +138,55 @@ void testIsClientErrorBoundaries() { assertFalse(new ConductorClientException(499, "").isClientError()); assertFalse(new ConductorClientException(500, "").isClientError()); } + + @Test + @DisplayName("An error is indeterminate unless something proves otherwise") + void isDefinite_byDefault_isFalse() { + var e = new ConductorClientException("boom"); + assertFalse(e.isDefinite(), "the safe default is indeterminate"); + } + + @Test + @DisplayName("A plain 4xx means the server rejected the request without applying it") + void definiteFor_clientErrors_isTrue() { + assertTrue(ApiException.definiteFor(400)); + assertTrue(ApiException.definiteFor(401)); + assertTrue(ApiException.definiteFor(403)); + assertTrue(ApiException.definiteFor(404)); + assertTrue(ApiException.definiteFor(405)); + assertTrue(ApiException.definiteFor(415)); + } + + @Test + @DisplayName("A 5xx may have applied the write before failing, so it stays indeterminate") + void definiteFor_serverErrors_isFalse() { + assertFalse(ApiException.definiteFor(500)); + assertFalse(ApiException.definiteFor(502)); + assertFalse(ApiException.definiteFor(503)); + assertFalse(ApiException.definiteFor(504)); + } + + @Test + @DisplayName("Conductor can return these four 4xx codes after it has already written") + void definiteFor_postWriteClientErrors_isFalse() { + assertFalse(ApiException.definiteFor(408), "the server may have begun processing a partial request"); + assertFalse(ApiException.definiteFor(409), "FAIL_ON_RUNNING throws CONFLICT after createOnly, without removing the row"); + assertFalse(ApiException.definiteFor(423), "LOCK is returned on paths that invite a retry"); + assertFalse(ApiException.definiteFor(429), "RATE_LIMITED is thrown after createOnly"); + } + + @Test + @DisplayName("No status means no response, which proves nothing") + void definiteFor_noStatus_isFalse() { + assertFalse(ApiException.definiteFor(0)); + assertFalse(ApiException.definiteFor(200)); + } + + @Test + @DisplayName("A server error body cannot talk the client into claiming definiteness") + void definite_isNotDeserializedFromTheResponseBody() throws Exception { + var mapper = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + var e = mapper.readValue("{\"definite\":true,\"code\":\"X\"}", ConductorClientException.class); + assertFalse(e.isDefinite(), "definiteness is the client's call, not the server's"); + } } From 7f8a040f3c099d9851d9e3629f45e88f822995b2 Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:27:36 -0300 Subject: [PATCH 2/7] test(client): pin the positive path of isDefinite() Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../client/exception/ConductorClientExceptionTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java index d05d2a225..38550c97c 100644 --- a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java +++ b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java @@ -146,6 +146,14 @@ void isDefinite_byDefault_isFalse() { assertFalse(e.isDefinite(), "the safe default is indeterminate"); } + @Test + @DisplayName("The flag round-trips, so the getter cannot quietly become a constant") + void setDefinite_thenIsDefinite_isTrue() { + var e = new ConductorClientException("boom"); + e.setDefinite(true); + assertTrue(e.isDefinite()); + } + @Test @DisplayName("A plain 4xx means the server rejected the request without applying it") void definiteFor_clientErrors_isTrue() { From 0c6773b7a1885a7c6dfca2ad689688616f6755c3 Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:31:42 -0300 Subject: [PATCH 3/7] feat(client): classify definiteness at the ConductorClient throw sites Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../client/http/ConductorClient.java | 13 +- .../conductor/client/http/WorkflowClient.java | 4 +- .../http/ClientErrorDefinitenessTest.java | 116 ++++++++++++++++++ 3 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java diff --git a/conductor-client/src/main/java/com/netflix/conductor/client/http/ConductorClient.java b/conductor-client/src/main/java/com/netflix/conductor/client/http/ConductorClient.java index aaa3073d3..2f897ee9d 100644 --- a/conductor-client/src/main/java/com/netflix/conductor/client/http/ConductorClient.java +++ b/conductor-client/src/main/java/com/netflix/conductor/client/http/ConductorClient.java @@ -53,6 +53,8 @@ import com.netflix.conductor.client.metrics.PayloadKind; import com.netflix.conductor.common.config.ObjectMapperProvider; +import io.orkes.conductor.client.http.ApiException; + import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JavaType; @@ -393,23 +395,30 @@ private RequestBody serialize(String contentType, @NotNull Object body) { return RequestBody.create(content, MediaType.parse(contentType)); } // Existing behavior for unsupported non-JSON, non-text types - throw new ConductorClientException("Content type \"" + contentType + "\" is not supported"); + ConductorClientException unsupported = + new ConductorClientException("Content type \"" + contentType + "\" is not supported"); + unsupported.setDefinite(true); + throw unsupported; } protected T handleResponse(Response response, Type returnType) { if (!response.isSuccessful()) { String respBody = bodyAsString(response); + boolean definite = ApiException.definiteFor(response.code()); try { ConductorClientException exception = objectMapper.readValue(respBody, ConductorClientException.class); exception.setStatus(response.code()); + exception.setDefinite(definite); throw exception; } catch (JsonProcessingException jpe) { // Ignore } - throw new ConductorClientException(response.message(), + ConductorClientException exception = new ConductorClientException(response.message(), response.code(), response.headers().toMultimap(), respBody); + exception.setDefinite(definite); + throw exception; } try { diff --git a/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java b/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java index 56e4d6cf4..731ece941 100644 --- a/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java +++ b/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java @@ -237,7 +237,9 @@ public void checkAndUploadToExternalStorage(StartWorkflowRequest startWorkflowRe * 1024L)) { String errorMsg = String.format("Input payload larger than the allowed threshold of: %d KB", conductorClientConfiguration.getWorkflowInputPayloadThresholdKB()); - throw new ConductorClientException(errorMsg); + ConductorClientException tooLarge = new ConductorClientException(errorMsg); + tooLarge.setDefinite(true); + throw tooLarge; } else { eventDispatcher.publish(new WorkflowPayloadUsedEvent(startWorkflowRequest.getName(), startWorkflowRequest.getVersion(), diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java new file mode 100644 index 000000000..8c2708a1e --- /dev/null +++ b/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java @@ -0,0 +1,116 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * 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 com.netflix.conductor.client.http; + +import java.io.IOException; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import com.netflix.conductor.client.exception.ConductorClientException; +import com.netflix.conductor.common.metadata.workflow.StartWorkflowRequest; + +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * A client error must not claim the request failed unless it did. Retrying an indeterminate start + * runs the workflow twice. + * + * @see jepsen-conductor#6 + */ +class ClientErrorDefinitenessTest { + + private MockWebServer server; + private WorkflowClient workflowClient; + + @BeforeEach + void setUp() throws IOException { + server = new MockWebServer(); + server.start(); + workflowClient = new WorkflowClient(new ConductorClient(server.url("/api").toString())); + } + + @AfterEach + void tearDown() throws IOException { + server.shutdown(); + Thread.interrupted(); + } + + @Test + @DisplayName("The server rejected the request: definite, safe to retry") + void badRequest_isDefinite() { + server.enqueue(new MockResponse().setResponseCode(400).setBody("bad request")); + + var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + + assertEquals(400, e.getStatus()); + assertTrue(e.isDefinite(), "a 400 means the workflow was never created"); + } + + @Test + @DisplayName("The server failed after reading the request: indeterminate") + void serverError_isNotDefinite() { + server.enqueue(new MockResponse().setResponseCode(500).setBody("boom")); + + var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + + assertEquals(500, e.getStatus()); + assertFalse(e.isDefinite(), "a 500 may have created the workflow before failing"); + } + + @Test + @DisplayName("The connection dropped after the request was read: indeterminate, this is the Jepsen case") + void connectionDroppedAfterRequest_isNotDefinite() { + server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AFTER_REQUEST)); + + var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + + assertFalse(e.isDefinite(), "the server may have created the workflow before the connection dropped"); + } + + @Test + @DisplayName("The connection dropped before the request was read: still indeterminate, OkHttp may have already sent it once") + void connectionDroppedAtStart_isNotDefinite() { + server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START)); + + var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + + assertFalse(e.isDefinite(), "a dropped connection never proves the request was not delivered"); + } + + @Test + @DisplayName("A 2xx with no workflow id: the start was applied, the outcome is unknown") + void emptySuccessBody_isNotDefinite() { + server.enqueue(new MockResponse().setResponseCode(200).setBody("")); + + var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + + assertFalse(e.isDefinite(), "the server accepted the request; only the id was lost"); + } + + private StartWorkflowRequest startRequest() { + var request = new StartWorkflowRequest(); + request.setName("definiteness_test"); + request.setVersion(1); + return request; + } +} From 88dc0a964aac4618c98879c30769d59cb67422a5 Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:36:11 -0300 Subject: [PATCH 4/7] test(client): pin the dropped-connection scenarios in ClientErrorDefinitenessTest Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../conductor/client/http/ClientErrorDefinitenessTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java index 8c2708a1e..2cfbb74eb 100644 --- a/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java +++ b/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java @@ -13,6 +13,7 @@ package com.netflix.conductor.client.http; import java.io.IOException; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -28,6 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -79,11 +81,13 @@ void serverError_isNotDefinite() { @Test @DisplayName("The connection dropped after the request was read: indeterminate, this is the Jepsen case") - void connectionDroppedAfterRequest_isNotDefinite() { + void connectionDroppedAfterRequest_isNotDefinite() throws InterruptedException { server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AFTER_REQUEST)); var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + assertNotNull(server.takeRequest(5, TimeUnit.SECONDS), "the server received the request"); + assertEquals(0, e.getStatus(), "no response status ever came back"); assertFalse(e.isDefinite(), "the server may have created the workflow before the connection dropped"); } @@ -94,6 +98,7 @@ void connectionDroppedAtStart_isNotDefinite() { var e = assertThrows(ConductorClientException.class, () -> workflowClient.startWorkflow(startRequest())); + assertEquals(0, e.getStatus(), "no response status ever came back"); assertFalse(e.isDefinite(), "a dropped connection never proves the request was not delivered"); } From 774f563c698e1e95e74e54c9720171a28cf0174e Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:39:18 -0300 Subject: [PATCH 5/7] test(client): pin the definiteness contract on the legacy ApiClient Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../client/ApiClientDefinitenessTest.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 conductor-client/src/test/java/io/orkes/conductor/client/ApiClientDefinitenessTest.java diff --git a/conductor-client/src/test/java/io/orkes/conductor/client/ApiClientDefinitenessTest.java b/conductor-client/src/test/java/io/orkes/conductor/client/ApiClientDefinitenessTest.java new file mode 100644 index 000000000..f5d7b2f77 --- /dev/null +++ b/conductor-client/src/test/java/io/orkes/conductor/client/ApiClientDefinitenessTest.java @@ -0,0 +1,89 @@ +/* + * Copyright 2026 Conductor Authors. + *

+ * 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 io.orkes.conductor.client; + +import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import com.netflix.conductor.client.exception.ConductorClientException; + +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The legacy {@link ApiClient} duplicates {@code execute(Call, Type)} rather than delegating, so the + * definiteness contract has to be pinned on its own code path, not through {@code WorkflowClient}. + * + * @see jepsen-conductor#6 + */ +class ApiClientDefinitenessTest { + + private MockWebServer server; + private ApiClient apiClient; + + @BeforeEach + void setUp() throws IOException { + server = new MockWebServer(); + server.start(); + apiClient = new ApiClient(server.url("/api").toString()); + } + + @AfterEach + void tearDown() throws IOException { + server.shutdown(); + Thread.interrupted(); + } + + @Test + @DisplayName("A rejected request is definite on the legacy client's own execute path") + void badRequest_isDefinite() { + server.enqueue(new MockResponse().setResponseCode(400).setBody("bad request")); + + var e = assertThrows(ConductorClientException.class, this::executeOnApiClient); + + assertEquals(400, e.getStatus()); + assertTrue(e.isDefinite(), "a 400 means the request was never applied"); + } + + @Test + @DisplayName("A dropped connection stays indeterminate on the legacy client's own execute path") + void connectionDroppedAfterRequest_isNotDefinite() throws InterruptedException { + server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AFTER_REQUEST)); + + var e = assertThrows(ConductorClientException.class, this::executeOnApiClient); + + assertNotNull(server.takeRequest(5, TimeUnit.SECONDS), "the server received the request"); + assertEquals(0, e.getStatus(), "no response status ever came back"); + assertFalse(e.isDefinite(), "the server may have applied it before the connection dropped"); + } + + // Builds and runs the call on the ApiClient itself, so its duplicated execute() is the one under test. + private void executeOnApiClient() { + var call = apiClient.buildCall("/workflow", "POST", List.of(), List.of(), "{}", Map.of()); + apiClient.execute(call, String.class); + } +} From 557dca14c167efcfc8349513915a9cee815f6021 Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:57:01 -0300 Subject: [PATCH 6/7] fix(client): exclude 402 from definiteFor, print definite in toString, align IOException sibling MetadataResource.save can throw PAYMENT_REQUIRED (402) after registerWorkflowDef already committed the write, so definiteFor(402) was wrongly telling callers a retry was safe. ApiException#toString() never printed the definite flag, so log readers still had no way to tell a logged failure's definiteness, which was the core ask behind this flag. The print is placed outside the status>0 guard so it still shows for the Jepsen status==0 case. checkAndUploadToExternalStorage's local JSON-serialization failure is a pre-transmission error like its payload-threshold sibling, so it now gets the same setDefinite(true). Also fixes a broken {@link #isRetryable()} javadoc reference (Lombok-generated, unresolvable). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../conductor/client/http/WorkflowClient.java | 4 +++- .../conductor/client/http/ApiException.java | 13 ++++++++++--- .../ConductorClientExceptionTest.java | 13 ++++++++++++- .../http/ClientErrorDefinitenessTest.java | 19 +++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java b/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java index 731ece941..0022ce40d 100644 --- a/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java +++ b/conductor-client/src/main/java/com/netflix/conductor/client/http/WorkflowClient.java @@ -261,7 +261,9 @@ public void checkAndUploadToExternalStorage(StartWorkflowRequest startWorkflowRe eventDispatcher.publish(new WorkflowStartedEvent(startWorkflowRequest.getName(), startWorkflowRequest.getVersion(), false, e)); - throw new ConductorClientException(e); + ConductorClientException serializationFailed = new ConductorClientException(e); + serializationFailed.setDefinite(true); + throw serializationFailed; } } diff --git a/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java b/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java index a14e13760..e3ab739c0 100644 --- a/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java +++ b/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java @@ -36,6 +36,7 @@ static boolean initPreferErrOverResponse() { private final static boolean PREFER_ERR_OVER_RESPONSE = initPreferErrOverResponse(); + private static final int HTTP_PAYMENT_REQUIRED = 402; private static final int HTTP_REQUEST_TIMEOUT = 408; private static final int HTTP_CONFLICT = 409; private static final int HTTP_LOCKED = 423; @@ -115,7 +116,7 @@ public boolean isClientError() { * may retry a request on a fresh route after a pooled connection fails mid-send, so "failed to * connect" can follow a request the server already received. * - *

This is not {@link #isRetryable()}. That one says whether trying again is worth it; this + *

This is not {@code isRetryable()}. That one says whether trying again is worth it; this * one says whether trying again can duplicate work. They are independent and often opposite: a * 503 is retryable and indeterminate at the same time. */ @@ -126,12 +127,16 @@ public boolean isDefinite() { /** * Whether an HTTP status proves the server rejected the request without applying it. * - *

Most 4xx codes qualify. Four do not: Conductor can return 408, 409, 423 and 429 after it - * has already written, so a caller that retried them could duplicate the work. + *

Most 4xx codes qualify. 402, 408, 409, 423 and 429 do not: Conductor can return these + * after it has already written, so a caller that retried them could duplicate the work. + * + *

This classification reflects the current server's behaviour and may change as the server + * changes; it is advisory, not a durable guarantee. */ public static boolean definiteFor(int status) { return status >= 400 && status < 500 + && status != HTTP_PAYMENT_REQUIRED && status != HTTP_REQUEST_TIMEOUT && status != HTTP_CONFLICT && status != HTTP_LOCKED @@ -175,6 +180,8 @@ public String toString() { builder.append(", retryable: ").append(retryable); } + builder.append(", definite: ").append(definite); + if (this.instance != null) { builder.append(", instance: ").append(instance); } diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java index 38550c97c..f629b3c40 100644 --- a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java +++ b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java @@ -175,8 +175,9 @@ void definiteFor_serverErrors_isFalse() { } @Test - @DisplayName("Conductor can return these four 4xx codes after it has already written") + @DisplayName("Conductor can return these five 4xx codes after it has already written") void definiteFor_postWriteClientErrors_isFalse() { + assertFalse(ApiException.definiteFor(402), "a definition can be written before replaceTags throws PAYMENT_REQUIRED"); assertFalse(ApiException.definiteFor(408), "the server may have begun processing a partial request"); assertFalse(ApiException.definiteFor(409), "FAIL_ON_RUNNING throws CONFLICT after createOnly, without removing the row"); assertFalse(ApiException.definiteFor(423), "LOCK is returned on paths that invite a retry"); @@ -190,6 +191,16 @@ void definiteFor_noStatus_isFalse() { assertFalse(ApiException.definiteFor(200)); } + @Test + @DisplayName("toString prints definite even with no status, the Jepsen case the status>0 guard would hide it in") + void toString_printsDefinite_evenWithNoStatus() { + var e = new ConductorClientException("connection failed"); + e.setDefinite(false); + + assertEquals(0, e.getStatus(), "this is the no-response case the guard must not hide definite behind"); + assertTrue(e.toString().contains("definite: false"), "definite must print outside the status>0 guard"); + } + @Test @DisplayName("A server error body cannot talk the client into claiming definiteness") void definite_isNotDeserializedFromTheResponseBody() throws Exception { diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java index 2cfbb74eb..a1ad07b93 100644 --- a/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java +++ b/conductor-client/src/test/java/com/netflix/conductor/client/http/ClientErrorDefinitenessTest.java @@ -112,10 +112,29 @@ void emptySuccessBody_isNotDefinite() { assertFalse(e.isDefinite(), "the server accepted the request; only the id was lost"); } + @Test + @DisplayName("Local JSON serialization failure before any request is sent: definite, same as the payload-threshold sibling") + void inputSerializationFailure_isDefinite() { + var request = startRequest(); + request.getInput().put("boom", new UnserializableValue()); + + var e = assertThrows(ConductorClientException.class, + () -> workflowClient.checkAndUploadToExternalStorage(request)); + + assertTrue(e.isDefinite(), "the input was never sent; serialization failed locally first"); + } + private StartWorkflowRequest startRequest() { var request = new StartWorkflowRequest(); request.setName("definiteness_test"); request.setVersion(1); return request; } + + // A bean whose getter throws, so ObjectMapper#writeValue fails with an IOException subtype. + public static class UnserializableValue { + public String getValue() { + throw new RuntimeException("cannot serialize"); + } + } } From 664c980171139262f9986af3ce8874d77cc017f8 Mon Sep 17 00:00:00 2001 From: Miguel Prieto Date: Thu, 1 Oct 2026 20:59:13 -0300 Subject: [PATCH 7/7] fix(client): open the toString brace unconditionally so definite renders cleanly at status==0 The prior change appended ", definite: ..." unconditionally after a brace that was only opened inside the status>0 guard, so the status==0 case (the Jepsen case this flag exists for) rendered a stray leading comma with no opening brace. Open " {" unconditionally and move the separator inside the guard instead, so status>0 renders byte-identical to before and status==0 now renders " {definite: false}". Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JvEmz7CHxsyLEmhWwPWm7F --- .../conductor/client/http/ApiException.java | 9 +++++---- .../ConductorClientExceptionTest.java | 20 ++++++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java b/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java index e3ab739c0..2bebacc1c 100644 --- a/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java +++ b/conductor-client/src/main/java/io/orkes/conductor/client/http/ApiException.java @@ -171,16 +171,17 @@ public String toString() { builder.append(getMessage()); } + builder.append(" {"); + if (status > 0) { - builder.append(" {status=").append(status); + builder.append("status=").append(status); if (this.code != null) { builder.append(", code='").append(code).append("'"); } - - builder.append(", retryable: ").append(retryable); + builder.append(", retryable: ").append(retryable).append(", "); } - builder.append(", definite: ").append(definite); + builder.append("definite: ").append(definite); if (this.instance != null) { builder.append(", instance: ").append(instance); diff --git a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java index f629b3c40..29955178e 100644 --- a/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java +++ b/conductor-client/src/test/java/com/netflix/conductor/client/exception/ConductorClientExceptionTest.java @@ -192,13 +192,27 @@ void definiteFor_noStatus_isFalse() { } @Test - @DisplayName("toString prints definite even with no status, the Jepsen case the status>0 guard would hide it in") - void toString_printsDefinite_evenWithNoStatus() { + @DisplayName("toString renders a clean brace with no status, the Jepsen case the status>0 guard would hide definite in") + void toString_withNoStatus_rendersDefiniteWithoutGarbage() { var e = new ConductorClientException("connection failed"); e.setDefinite(false); assertEquals(0, e.getStatus(), "this is the no-response case the guard must not hide definite behind"); - assertTrue(e.toString().contains("definite: false"), "definite must print outside the status>0 guard"); + assertEquals( + "com.netflix.conductor.client.exception.ConductorClientException: connection failed {definite: false}", + e.toString()); + } + + @Test + @DisplayName("toString with a status keeps the existing status>0 shape, now followed by definite") + void toString_withStatus_rendersStatusThenDefinite() { + var e = new ConductorClientException(400, "bad request"); + e.setRetryable(true); + e.setDefinite(true); + + assertEquals( + "com.netflix.conductor.client.exception.ConductorClientException: bad request {status=400, retryable: true, definite: true}", + e.toString()); } @Test