Resolve #140: SchedulerClient.pauseSchedule() fails: 'Request method GET is not supported' (500) — GET-then-PUT fallback mishandles server response

conductor-client/src/main/java/io/orkes/conductor/client/http/SchedulerResource.java

@@ -13,6 +13,7 @@
 package io.orkes.conductor.client.http;
 
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import com.netflix.conductor.client.exception.ConductorClientException;
@@ -31,6 +32,9 @@
 
 public class SchedulerResource {
 
+    private static final int HTTP_METHOD_NOT_ALLOWED = 405;
+    private static final String METHOD_NOT_SUPPORTED_MARKER = "request method 'get' is not supported";
+
     private final ConductorClient client;
 
     public SchedulerResource(ConductorClient client) {
@@ -167,20 +171,27 @@ public void resumeSchedule(String name) {
     /**
      * Enterprise scheduler endpoints accept GET while OSS accepts PUT. Retry only
      * a method-not-allowed response so application and authentication failures
-     * retain their original behavior.
+     * retain their original behavior. This also handles OSS servers that return
+     * an HTTP 500 with a "method not supported" message instead of a clean 405.
      */
     private void executeGetThenPutOnMethodNotAllowed(
             ConductorClientRequest getRequest, ConductorClientRequest putRequest) {
         try {
             client.execute(getRequest);
         } catch (ConductorClientException e) {
-            if (e.getStatus() != 405) {
+            if (!isMethodNotSupported(e)) {
                 throw e;
             }
             client.execute(putRequest);
         }
     }
 
+    private boolean isMethodNotSupported(ConductorClientException e) {
+        return e.getStatus() == HTTP_METHOD_NOT_ALLOWED
+                || (e.getMessage() != null
+                        && e.getMessage().toLowerCase(Locale.ROOT).contains(METHOD_NOT_SUPPORTED_MARKER));
+    }
+
     public void saveSchedule(SaveScheduleRequest saveScheduleRequest) {
         ConductorClientRequest request = ConductorClientRequest.builder()
                 .method(Method.POST)

conductor-client/src/test/java/io/orkes/conductor/client/http/SchedulerResourceTest.java

@@ -92,9 +92,37 @@ void resumeScheduleUsesTheSameGetThenPutFallback() throws InterruptedException {
         assertEquals("PUT", takeRequest().getMethod());
     }
 
+    @Test
+    void pauseScheduleRetriesWithPutOnMethodNotSupported500() throws InterruptedException {
+        server.enqueue(new MockResponse().setResponseCode(500).setBody("Request method 'GET' is not supported"));
+        server.enqueue(new MockResponse().setResponseCode(204));
+
+        schedulerClient.pauseSchedule("nightly");
+
+        RecordedRequest get = takeRequest();
+        RecordedRequest put = takeRequest();
+        assertEquals("GET", get.getMethod());
+        assertEquals("PUT", put.getMethod());
+        assertEquals("/api/scheduler/schedules/nightly/pause", put.getPath());
+    }
+
+    @Test
+    void resumeScheduleRetriesWithPutOnMethodNotSupported500() throws InterruptedException {
+        server.enqueue(new MockResponse().setResponseCode(500).setBody("Request method 'GET' is not supported"));
+        server.enqueue(new MockResponse().setResponseCode(204));
+
+        schedulerClient.resumeSchedule("nightly");
+
+        RecordedRequest get = takeRequest();
+        RecordedRequest put = takeRequest();
+        assertEquals("GET", get.getMethod());
+        assertEquals("PUT", put.getMethod());
+        assertEquals("/api/scheduler/schedules/nightly/resume", put.getPath());
+    }
+
     @Test
     void pauseScheduleDoesNotRetryNon405Failures() throws InterruptedException {
-        server.enqueue(new MockResponse().setResponseCode(500));
+        server.enqueue(new MockResponse().setResponseCode(500).setBody("Internal server error"));
 
         assertThrows(RuntimeException.class, () -> schedulerClient.pauseSchedule("nightly"));
 

docs/design/architecture.md

@@ -0,0 +1,161 @@
+# Architecture — Fix `SchedulerClient.pauseSchedule()` GET-then-PUT fallback (Issue #140)
+
+## 1. Overview
+
+`SchedulerResource` in the Java client supports two server flavors for the
+schedule pause/resume endpoints:
+
+- **Enterprise (Orkes)** servers accept `GET /scheduler/schedules/{name}/pause`.
+- **OSS Conductor** servers only accept `PUT` on the same path.
+
+To work against both without a configuration flag, the client optimistically
+issues a `GET` first and, if the server reports that `GET` is not allowed for
+that route, retries the identical call with `PUT`. This is implemented by
+`SchedulerResource.executeGetThenPutOnMethodNotAllowed(...)`.
+
+### The bug
+
+The fallback only recognizes a **clean HTTP `405 Method Not Allowed`** as the
+signal to retry with `PUT`. Conductor OSS `3.32.0-rc.10` (Spring MVC) does not
+return `405` for this route — it returns **HTTP `500`** with the body
+`Request method 'GET' is not supported`. Because the status is `500` and not
+`405`, the fallback rethrows the exception instead of proceeding to the `PUT`,
+so `pauseSchedule(...)` (and `resumeSchedule(...)`) fail against OSS servers.
+
+```
+com.netflix.conductor.client.exception.ConductorClientException:
+    Request method 'GET' is not supported {status=500, retryable: false}
+  at ...SchedulerResource.executeGetThenPutOnMethodNotAllowed(SchedulerResource.java:175)
+  at ...SchedulerResource.pauseSchedule(SchedulerResource.java:125)
+```
+
+### The fix (scope)
+
+Broaden the retry predicate in `executeGetThenPutOnMethodNotAllowed` so that it
+treats the server's "method not supported" signal as a trigger to fall through
+to the `PUT`, whether it arrives as a genuine `405` **or** as a `500` whose
+message indicates the request method is not supported. All other failures
+(auth, validation, unrelated application `500`s, network) keep their current
+behavior and propagate unchanged.
+
+This is a **minimal, focused** change confined to one private method plus its
+tests. No public API, no new files, no dependency changes.
+
+## 2. Tech stack
+
+| Concern            | Choice                                                             |
+|--------------------|--------------------------------------------------------------------|
+| Language           | Java 21                                                            |
+| Module             | `conductor-client`                                                 |
+| HTTP client        | `com.netflix.conductor.client.http.ConductorClient` (OkHttp based) |
+| Exceptions         | `com.netflix.conductor.client.exception.ConductorClientException`  |
+| Tests              | JUnit 5 (`org.junit.jupiter`) + `okhttp3.mockwebserver.MockWebServer` |
+| Build              | Gradle                                                             |
+
+## 3. Module / file layout
+
+Only **existing** files are touched. No new source files are created.
+
+| File | Change | Responsibility |
+|------|--------|----------------|
+| `conductor-client/src/main/java/io/orkes/conductor/client/http/SchedulerResource.java` | **MODIFY** | Add the private helper `isMethodNotSupported(ConductorClientException)` and the two detection constants; update `executeGetThenPutOnMethodNotAllowed(...)` to use the helper. |
+| `conductor-client/src/test/java/io/orkes/conductor/client/http/SchedulerResourceTest.java` | **MODIFY** | Add regression tests covering the `500 "Request method 'GET' is not supported"` fallthrough for both `pauseSchedule` and `resumeSchedule`, and confirm unrelated `500`s still propagate. |
+
+No changes are required in
+`agent-examples/.../Example99ScheduledAgent.java`; it is the reproduction, not
+part of the fix.
+
+## 4. Shared contracts (reused verbatim by every component)
+
+These names, signatures, and constants are the single source of truth. The
+supporting docs and the tests reuse them exactly.
+
+### 4.1 Exception surface (existing — do not change)
+
+`ConductorClientException extends ApiException`. Relevant accessors used by the
+fix:
+
+```java
+int getStatus();          // HTTP status code, e.g. 405 or 500
+String getMessage();      // resolves to responseBody when present (see ApiException.getMessage)
+```
+
+`ApiException.getMessage()` returns the `responseBody` when it is non-blank, so
+for the OSS failure it yields the string `Request method 'GET' is not supported`.
+
+### 4.2 Detection constants (new — private to `SchedulerResource`)
+
+```java
+private static final int HTTP_METHOD_NOT_ALLOWED = 405;
+
+// Substring Spring MVC emits when a route rejects the HTTP verb.
+// Matched case-insensitively against ConductorClientException.getMessage().
+private static final String METHOD_NOT_SUPPORTED_MARKER = "request method 'get' is not supported";
+```
+
+### 4.3 Detection helper (new — private to `SchedulerResource`)
+
+```java
+/**
+ * True when the server rejected the GET because the HTTP verb is not
+ * supported for the route. Recognizes both a clean 405 and the OSS
+ * Conductor behavior of returning 500 with a
+ * "Request method 'GET' is not supported" body.
+ */
+private boolean isMethodNotSupported(ConductorClientException e);
+```
+
+Predicate (exact semantics):
+
+```
+message = e.getMessage()
+
+isMethodNotSupported(e) ==
+    e.getStatus() == HTTP_METHOD_NOT_ALLOWED
+    || (message != null
+        && message.toLowerCase(Locale.ROOT).contains(METHOD_NOT_SUPPORTED_MARKER))
+```
+
+`java.util.Locale` is imported for the case-insensitive comparison.
+
+### 4.4 Fallback method (existing signature — body updated)
+
+```java
+private void executeGetThenPutOnMethodNotAllowed(
+        ConductorClientRequest getRequest, ConductorClientRequest putRequest);
+```
+
+New body:
+
+```java
+try {
+    client.execute(getRequest);
+} catch (ConductorClientException e) {
+    if (!isMethodNotSupported(e)) {
+        throw e;
+    }
+    client.execute(putRequest);
+}
+```
+
+### 4.5 Behavior contract
+
+| GET response from server                                     | Client action                     |
+|--------------------------------------------------------------|-----------------------------------|
+| `2xx` (e.g. `204`)                                          | Succeed. No `PUT`.                |
+| `405 Method Not Allowed`                                    | Retry with `PUT`.                 |
+| `500` with body containing `Request method 'GET' is not supported` | Retry with `PUT`.          |
+| Any other failure (`401`, `403`, `400`, unrelated `500`, IO) | Propagate the original exception. |
+
+The `PUT` retry itself is **not** wrapped: if the `PUT` fails, that exception
+propagates as-is (unchanged from current behavior).
+
+Both `pauseSchedule(String, String)` and `resumeSchedule(String)` route through
+`executeGetThenPutOnMethodNotAllowed`, so both benefit from the fix identically.
+
+## 5. Non-goals
+
+- No change to the public `SchedulerClient` / `OrkesSchedulerClient` API.
+- No configuration flag to force GET vs PUT (the auto-fallback stays).
+- No change to any other `SchedulerResource` endpoint or to `ConductorClient`.
+- No change to how exceptions are constructed or to `ApiException`.

docs/design/data-model.md

@@ -0,0 +1,119 @@
+# Data Model & Request Flow — Scheduler pause/resume fallback (Issue #140)
+
+This document details the request/response data and the decision flow for the
+GET-then-PUT fallback described in `architecture.md`. All names, constants, and
+signatures are those defined in `architecture.md` §4.
+
+## 1. Endpoints involved
+
+| Operation | Method (Enterprise) | Method (OSS) | Path |
+|-----------|---------------------|--------------|------|
+| pause     | `GET`               | `PUT`        | `/scheduler/schedules/{name}/pause`  |
+| resume    | `GET`               | `PUT`        | `/scheduler/schedules/{name}/resume` |
+
+`pause` additionally carries an optional `reason` query parameter. Per the
+existing behavior, `reason` is set on **both** the prepared `GET` and `PUT`
+requests before the fallback runs.
+
+## 2. Server response shapes
+
+There is no request/response body model to add — the fix keys entirely off the
+HTTP status and the error message already surfaced by
+`ConductorClientException`.
+
+### 2.1 Enterprise server (GET accepted)
+
+```
+HTTP/1.1 204 No Content
+```
+
+→ Client returns normally; no `PUT` is issued.
+
+### 2.2 OSS server (GET rejected) — the case Issue #140 fixes
+
+```
+HTTP/1.1 500 Internal Server Error
+Content-Type: text/plain
+
+Request method 'GET' is not supported
+```
+
+The `ConductorClient` maps this to:
+
+```
+ConductorClientException {
+    status       = 500
+    responseBody = "Request method 'GET' is not supported"
+    getMessage() -> "Request method 'GET' is not supported"
+}
+```
+
+### 2.3 Older / spec-compliant server (GET rejected)
+
+```
+HTTP/1.1 405 Method Not Allowed
+```
+
+```
+ConductorClientException { status = 405, ... }
+```
+
+## 3. Decision flow
+
+```
+pauseSchedule(name, reason) / resumeSchedule(name)
+        |
+        v
+build getRequest (GET) + putRequest (PUT)   // reason applied to both for pause
+        |
+        v
+executeGetThenPutOnMethodNotAllowed(getRequest, putRequest)
+        |
+        +-- client.execute(getRequest)
+        |        |
+        |        +-- success (2xx) --------------------> return
+        |        |
+        |        +-- throws ConductorClientException e
+        |                 |
+        |                 v
+        |        isMethodNotSupported(e) ?
+        |          status == 405                                   -> true
+        |          OR getMessage() contains                        -> true
+        |             "request method 'get' is not supported"
+        |             (case-insensitive)
+        |          else                                            -> false
+        |                 |
+        |     true -------+------- false
+        |      |                     |
+        |      v                     v
+        |  client.execute(putRequest)   throw e   // propagate unchanged
+        |      |
+        |      +-- success -> return
+        |      +-- failure -> propagate PUT exception (unchanged)
+```
+
+## 4. Decision matrix (authoritative)
+
+Mirrors `architecture.md` §4.5. This is the exact truth table the
+implementation and tests must satisfy.
+
+| # | GET result                                         | `isMethodNotSupported` | PUT issued? | Outcome                     |
+|---|----------------------------------------------------|------------------------|-------------|-----------------------------|
+| 1 | `204`                                              | n/a                    | no          | success                     |
+| 2 | `405`                                              | `true`                 | yes         | PUT outcome                 |
+| 3 | `500` body `Request method 'GET' is not supported` | `true`                 | yes         | PUT outcome                 |
+| 4 | `500` unrelated body (e.g. `boom`)                 | `false`                | no          | original exception rethrown |
+| 5 | `401` / `403` / `400`                              | `false`                | no          | original exception rethrown |
+
+Row 4 is the guardrail that keeps the fix minimal: only the specific
+"method not supported" `500` falls through; every other `500` still fails fast.
+This preserves the existing "do not retry non-405 failures" contract in spirit
+(that test is updated to use an unrelated `500` body — see `testing.md`).
+
+## 5. Why match on message, not just status
+
+A blanket "retry any `500` with `PUT`" would mask genuine server errors and
+double-fire mutating requests. Matching the Spring MVC marker string
+`request method 'get' is not supported` (case-insensitive, via
+`METHOD_NOT_SUPPORTED_MARKER`) restricts the fallthrough to exactly the
+verb-rejection condition, keeping the behavior safe and targeted.