fix(webserver): webhook hang indefinitly for 404 status

When we send a 404 status by retuning HttpReponse.notFound(), the webhook hang forever for POST request (but works for GET). Throwing an HttpStatusException fix the issue, it seems to be caused by a bug in Micronaut, I will try to create a reproducer and open an issue upstream by meanwhile this fixes the issue.
kestra-io · Sep 5, 2024 · 788dc13 · 788dc13
1 parent 1b0e3cb
commit 788dc13
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 3 deletions.
diff --git a/webserver/src/main/java/io/kestra/webserver/controllers/ErrorController.java b/webserver/src/main/java/io/kestra/webserver/controllers/ErrorController.java
@@ -13,6 +13,7 @@
 import io.micronaut.http.annotation.Controller;
 import io.micronaut.http.annotation.Error;
 import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.http.exceptions.HttpStatusException;
 import io.micronaut.http.hateoas.JsonError;
 import io.micronaut.http.hateoas.Link;
 import io.micronaut.web.router.exceptions.UnsatisfiedBodyRouteException;
@@ -138,6 +139,11 @@ public HttpResponse<JsonError> error(HttpRequest<?> request, UnsatisfiedBodyRout
         return jsonError(request, e, HttpStatus.UNPROCESSABLE_ENTITY, "Invalid route params");
     }
 
+    @Error(global = true)
+    public HttpResponse<JsonError> error(HttpRequest<?> request, HttpStatusException e) {
+        return jsonError(request, e, e.getStatus(), e.getStatus().getReason());
+    }
+
     @Error(global = true)
     public HttpResponse<JsonError> error(HttpRequest<?> request, Throwable e) {
         return jsonError(request, e, HttpStatus.INTERNAL_SERVER_ERROR, "Internal server error");

diff --git a/webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java b/webserver/src/main/java/io/kestra/webserver/controllers/api/ExecutionController.java
@@ -459,7 +459,7 @@ protected HttpResponse<Execution> webhook(
         HttpRequest<String> request
     ) {
         if (maybeFlow.isEmpty()) {
-            return HttpResponse.notFound();
+            throw new HttpStatusException(HttpStatus.NOT_FOUND, "Flow not found");
         }
 
         var flow = maybeFlow.get();
@@ -490,13 +490,13 @@ protected HttpResponse<Execution> webhook(
             .findFirst();
 
         if (webhook.isEmpty()) {
-            return HttpResponse.notFound();
+            throw new HttpStatusException(HttpStatus.NOT_FOUND, "Webhook not found");
         }
 
         Optional<Execution> execution = webhook.get().evaluate(request, flow);
 
         if (execution.isEmpty()) {
-            return HttpResponse.notFound();
+            throw new HttpStatusException(HttpStatus.NOT_FOUND, "No execution triggered");
         }
 
         var result = execution.get();

diff --git a/webserver/src/test/java/io/kestra/webserver/controllers/api/ExecutionControllerTest.java b/webserver/src/test/java/io/kestra/webserver/controllers/api/ExecutionControllerTest.java
@@ -717,6 +717,70 @@ void webhook() {
 
     }
 
+    @Test
+    void webhookFlowNotFound() {
+        HttpClientResponseException exception = assertThrows(HttpClientResponseException.class,
+            () -> client.toBlocking().retrieve(
+                HttpRequest
+                    .POST(
+                        "/api/v1/executions/webhook/not-found/webhook/not-found?name=john&age=12&age=13",
+                        ImmutableMap.of("a", 1, "b", true)
+                    ),
+                Execution.class
+            )
+        );
+        assertThat(exception.getStatus(), is(HttpStatus.NOT_FOUND));
+        assertThat(exception.getMessage(), containsString("Not Found: Flow not found"));
+
+        exception = assertThrows(HttpClientResponseException.class,
+            () -> client.toBlocking().retrieve(
+                HttpRequest
+                    .PUT(
+                        "/api/v1/executions/webhook/not-found/webhook/not-found?name=john&age=12&age=13",
+                        Collections.singletonList(ImmutableMap.of("a", 1, "b", true))
+                    ),
+                Execution.class
+            )
+        );
+        assertThat(exception.getStatus(), is(HttpStatus.NOT_FOUND));
+        assertThat(exception.getMessage(), containsString("Not Found: Flow not found"));
+
+        exception = assertThrows(HttpClientResponseException.class,
+            () -> client.toBlocking().retrieve(
+                HttpRequest
+                    .POST(
+                        "/api/v1/executions/webhook/not-found/webhook/not-found?name=john&age=12&age=13",
+                        "bla"
+                    ),
+                Execution.class
+            )
+        );
+        assertThat(exception.getStatus(), is(HttpStatus.NOT_FOUND));
+        assertThat(exception.getMessage(), containsString("Not Found: Flow not found"));
+
+        exception = assertThrows(HttpClientResponseException.class,
+            () -> client.toBlocking().retrieve(
+                GET("/api/v1/executions/webhook/not-found/webhook/not-found?name=john&age=12&age=13"),
+                Execution.class
+            )
+        );
+        assertThat(exception.getStatus(), is(HttpStatus.NOT_FOUND));
+        assertThat(exception.getMessage(), containsString("Not Found: Flow not found"));
+
+        exception = assertThrows(HttpClientResponseException.class,
+            () -> client.toBlocking().retrieve(
+                HttpRequest
+                    .POST(
+                        "/api/v1/executions/webhook/not-found/webhook/not-found?name=john&age=12&age=13",
+                        "{\\\"a\\\":\\\"\\\",\\\"b\\\":{\\\"c\\\":{\\\"d\\\":{\\\"e\\\":\\\"\\\",\\\"f\\\":\\\"1\\\"}}}}"
+                    ),
+                Execution.class
+            )
+        );
+        assertThat(exception.getStatus(), is(HttpStatus.NOT_FOUND));
+        assertThat(exception.getMessage(), containsString("Not Found: Flow not found"));
+    }
+
     @Test
     void webhookDynamicKey() {
         Execution execution = client.toBlocking().retrieve(