fix(): ignore loading bar accordingly

easyops-cn · Aug 28, 2023 · 51b68bf · 51b68bf
1 parent ca53f6d
commit 51b68bf
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 55 deletions.
diff --git a/packages/brick-container/src/bootstrap.ts b/packages/brick-container/src/bootstrap.ts
@@ -1,6 +1,6 @@
 // istanbul ignore file
 import { createRuntime, httpErrorToString } from "@next-core/runtime";
-import { http, HttpError, HttpResponse } from "@next-core/http";
+import { HttpRequestConfig, http } from "@next-core/http";
 import { i18n } from "@next-core/i18n";
 import {
   flowApi,
@@ -19,9 +19,7 @@ import { getSpanId } from "./utils.js";
 import { listen } from "./preview/listen.js";
 
 http.interceptors.request.use((config) => {
-  if (!config.options?.interceptorParams?.ignoreLoadingBar) {
-    window.dispatchEvent(new Event("request.start"));
-  }
+  dispatchRequestEventByConfig("request.start", config);
 
   const headers = new Headers(config.options?.headers || {});
 
@@ -50,16 +48,22 @@ http.interceptors.request.use((config) => {
 });
 
 http.interceptors.response.use(
-  function (response: HttpResponse) {
-    window.dispatchEvent(new Event("request.end"));
+  function (response, config) {
+    dispatchRequestEventByConfig("request.end", config);
     return response;
   },
-  function (error: HttpError) {
-    window.dispatchEvent(new Event("request.end"));
+  function (error, config) {
+    dispatchRequestEventByConfig("request.end", config);
     return Promise.reject(error);
   }
 );
 
+function dispatchRequestEventByConfig(type: string, config: HttpRequestConfig) {
+  if (!config.options?.interceptorParams?.ignoreLoadingBar) {
+    window.dispatchEvent(new Event(type));
+  }
+}
+
 const loadingBar = document.querySelector("#global-loading-bar")!;
 loadingBar.classList.add("rendered");
 

diff --git a/packages/http/src/__snapshots__/http.spec.ts.snap b/packages/http/src/__snapshots__/http.spec.ts.snap
@@ -398,13 +398,6 @@ exports[`http requestWithBody or simpleRequest PUT http://example.com with [] sh
 
 exports[`http should return http response object  1`] = `
 {
-  "config": {
-    "method": "GET",
-    "options": {
-      "observe": "response",
-    },
-    "url": "http://example.com",
-  },
   "data": {
     "foo": "bar",
   },

diff --git a/packages/http/src/http.spec.ts b/packages/http/src/http.spec.ts
@@ -25,9 +25,24 @@ type TestItem =
   return "<URLSearchParams> " + this.toString();
 };
 
+const requestInterceptor = jest.fn((conf) => conf);
+const responseInterceptor = jest.fn((response) => response);
+const responseRejectInterceptor = jest.fn((error) => Promise.reject(error));
+
 describe("http", () => {
-  afterEach(() => {
-    spyOnFetch.mockClear();
+  let requestInterceptorId: number;
+  let responseInterceptorId: number;
+  beforeAll(() => {
+    requestInterceptorId = http.interceptors.request.use(requestInterceptor);
+    responseInterceptorId = http.interceptors.response.use(
+      responseInterceptor,
+      responseRejectInterceptor
+    );
+  });
+
+  afterAll(() => {
+    http.interceptors.request.eject(requestInterceptorId);
+    http.interceptors.response.eject(responseInterceptorId);
   });
 
   const formData = new FormData();
@@ -127,6 +142,27 @@ describe("http", () => {
     });
 
     expect(spyOnFetch.mock.calls[0]).toMatchSnapshot();
+    expect(requestInterceptor).toBeCalledTimes(1);
+    expect(responseInterceptor).toBeCalledTimes(1);
+    expect(responseRejectInterceptor).not.toBeCalled();
+    expect(requestInterceptor).toBeCalledWith({
+      url: "http://example.com/for-good",
+      method: "GET",
+      options: {},
+    });
+    expect(responseInterceptor).toBeCalledWith(
+      {
+        status: 200,
+        statusText: "",
+        data: {},
+        headers: expect.any(Headers),
+      },
+      {
+        url: "http://example.com/for-good",
+        method: "GET",
+        options: {},
+      }
+    );
   });
 
   it("should work with getUrlWithParams", () => {
@@ -209,11 +245,19 @@ describe("http", () => {
 
   it("should throw a HttpParseError", async () => {
     __setReturnValue(Promise.resolve(new Response("non-json")));
-    expect.assertions(1);
+    expect.assertions(4);
     try {
       await http.get("http://example.com");
     } catch (e) {
       expect(e).toBeInstanceOf(HttpParseError);
+      expect(responseInterceptor).not.toBeCalled();
+      expect(responseRejectInterceptor).toBeCalledTimes(1);
+      expect(responseRejectInterceptor).toBeCalledWith(
+        e,
+        expect.objectContaining({
+          url: "http://example.com",
+        })
+      );
     }
   });
 

diff --git a/packages/http/src/http.ts b/packages/http/src/http.ts
@@ -35,8 +35,6 @@ export interface HttpConstructorOptions {
   adapter?: HttpAdapter;
 }
 
-// type NotNil<T> = T extends null ? never : T;
-
 function isNil(value: unknown): value is null | undefined {
   return value === undefined || value === null;
 }
@@ -66,16 +64,6 @@ export type HttpOptions = HttpCustomOptions & RequestInit;
 export const isHttpAbortError = (error: any) =>
   error instanceof DOMException && error.code === 20;
 
-const createError = (
-  error: HttpFetchError | HttpResponseError | HttpParseError | HttpAbortError,
-  config: HttpRequestConfig
-): HttpError => {
-  return {
-    error,
-    config,
-  };
-};
-
 const request = async <T>(
   url: string,
   init: RequestInit,
@@ -97,12 +85,9 @@ const request = async <T>(
       response = await fetch(url, init);
     } catch (e: any) {
       reject(
-        createError(
-          isHttpAbortError(e)
-            ? new HttpAbortError(e.toString())
-            : new HttpFetchError(e.toString()),
-          config
-        )
+        isHttpAbortError(e)
+          ? new HttpAbortError(e.toString())
+          : new HttpFetchError(e.toString())
       );
       return;
     }
@@ -114,9 +99,7 @@ const request = async <T>(
       } catch (e) {
         // Do nothing.
       }
-      reject(
-        createError(new HttpResponseError(response, responseJson), config)
-      );
+      reject(new HttpResponseError(response, responseJson));
       return;
     }
 
@@ -125,12 +108,9 @@ const request = async <T>(
       data = await response[responseType]();
     } catch (e: any) {
       reject(
-        createError(
-          isHttpAbortError(e)
-            ? new HttpAbortError(e.toString())
-            : new HttpParseError(response),
-          config
-        )
+        isHttpAbortError(e)
+          ? new HttpAbortError(e.toString())
+          : new HttpParseError(response)
       );
       return;
     }
@@ -339,16 +319,9 @@ class Http {
       );
     });
 
-    chain.push(
-      (response: HttpResponse) => {
-        return config.options?.observe === "response"
-          ? response
-          : response.data;
-      },
-      (error: HttpError) => {
-        return Promise.reject(error.error);
-      }
-    );
+    chain.push((response: HttpResponse) => {
+      return config.options?.observe === "response" ? response : response.data;
+    }, undefined);
 
     while (chain.length) {
       promise = promise.then(chain.shift(), chain.shift());