a way for async exceptions to get treated as such (#11726)

mechanism for the FE to handle exceptions when response is streaming

frontend/src/metabase/lib/api.js

@@ -179,20 +179,24 @@ export class Api extends EventEmitter {
           try {
             body = JSON.parse(body);
           } catch (e) {}
-          if (xhr.status >= 200 && xhr.status <= 299) {
+          let status = xhr.status;
+          if (status === 202 && body && body._status > 0) {
+            status = body._status;
+          }
+          if (status >= 200 && status <= 299) {
             if (options.transformResponse) {
               body = options.transformResponse(body, { data });
             }
             resolve(body);
           } else {
             reject({
-              status: xhr.status,
+              status: status,
               data: body,
               isCancelled: isCancelled,
             });
           }
           if (!options.noEvent) {
-            this.emit(xhr.status, url);
+            this.emit(status, url);
           }
         }
       };

frontend/test/metabase/lib/api.unit.spec.js

@@ -89,4 +89,19 @@ describe("api", () => {
     const response = await hello();
     expect(response).toEqual({ status: "ok" });
   });
+
+  it("should use _status from body when HTTP status is 202", async () => {
+    expect.assertions(2);
+    mock.get("/async-status", {
+      status: 202,
+      body: JSON.stringify({ _status: 400, message: "error message" }),
+    });
+    const asyncStatus = GET("/async-status");
+    try {
+      await asyncStatus();
+    } catch (error) {
+      expect(error.status).toBe(400);
+      expect(error.data.message).toBe("error message");
+    }
+  });
 });

src/metabase/api/card.clj

@@ -224,7 +224,7 @@
   (let [data-keys            [:dataset_query :description :display :name
                               :visualization_settings :collection_id :collection_position]
         card-data            (assoc (zipmap data-keys (map card-data data-keys))
-                               :creator_id api/*current-user-id*)
+                                    :creator_id api/*current-user-id*)
         result-metadata-chan (result-metadata-async dataset_query result_metadata metadata_checksum)
         out-chan             (a/chan 1)]
     (a/go
@@ -236,7 +236,7 @@
           (async.u/single-value-pipe (save-new-card-async! card-data) out-chan))
         (catch Throwable e
           (a/put! out-chan e)
-          (a/close! e))))
+          (a/close! out-chan))))
     ;; Return a channel
     out-chan))
 
@@ -258,7 +258,8 @@
   ;; check that we have permissions for the collection we're trying to save this card to, if applicable
   (collection/check-write-perms-for-collection collection_id)
   ;; Return a channel that can be used to fetch the results asynchronously
-  (create-card-async! body))
+  {:status 202
+   :body   (create-card-async! body)})
 
 
 ;;; ------------------------------------------------- Updating Cards -------------------------------------------------

src/metabase/async/api_response.clj

@@ -61,7 +61,9 @@
   (cond
     ;; An error has occurred, let the user know
     (instance? Throwable chunkk)
-    (json/generate-stream (:body (mw.exceptions/api-exception-response chunkk)) out)
+    (json/generate-stream (let [body (:body (mw.exceptions/api-exception-response chunkk))]
+                            (cond-> body (map? body) (assoc :_status 500)))
+                          out)
 
     ;; We've recevied the response, write it to the output stream and we're done
     (seqable? chunkk)

test/metabase/async/api_response_test.clj

@@ -121,9 +121,10 @@
 
 ;; An error should be written to the output stream
 (expect
-  {:message    "Input channel unexpectedly closed."
-   :type       "class java.lang.InterruptedException"
-   :stacktrace true}
+  {:message     "Input channel unexpectedly closed."
+   :type        "class java.lang.InterruptedException"
+   :_status 500
+   :stacktrace  true}
   (tu.async/with-chans [input-chan]
     (with-response [{:keys [os os-closed-chan]} input-chan]
       (a/close! input-chan)
@@ -176,7 +177,7 @@
 
 ;; If the message sent to input-chan is an Exception an appropriate response should be generated
 (expect
-  {:message "Broken", :type "class java.lang.Exception", :stacktrace true}
+  {:message "Broken", :type "class java.lang.Exception", :stacktrace true, :_status 500}
   (tu.async/with-chans [input-chan]
     (with-response [{:keys [os os-closed-chan]} input-chan]
       (a/>!! input-chan (Exception. "Broken"))
@@ -189,9 +190,10 @@
 ;; If we write a bad API endpoint and return a channel but never write to it, the request should be canceled after
 ;; `absolute-max-keepalive-ms`
 (expect
-  {:message    "No response after waiting 500.0 ms. Canceling request."
-   :type       "class java.util.concurrent.TimeoutException"
-   :stacktrace true}
+  {:message     "No response after waiting 500.0 ms. Canceling request."
+   :type        "class java.util.concurrent.TimeoutException"
+   :_status 500
+   :stacktrace  true}
   (with-redefs [async-response/absolute-max-keepalive-ms 500]
     (tu.async/with-chans [input-chan]
       (with-response [{:keys [os os-closed-chan]} input-chan]

test/metabase/query_processor_test/nested_queries_test.clj

@@ -521,7 +521,7 @@
                       Collection [dest-card-collection]]
         (perms/grant-collection-read-permissions!      (group/all-users) source-card-collection)
         (perms/grant-collection-readwrite-permissions! (group/all-users) dest-card-collection)
-        (save-card-via-API-with-native-source-query! 200 (data/db) source-card-collection dest-card-collection)
+        (save-card-via-API-with-native-source-query! 202 (data/db) source-card-collection dest-card-collection)
         :ok))))
 
 ;; however, if we do *not* have read permissions for the source Card's collection we shouldn't be allowed to save the