Do not fail on error 502 during restart actions (#1941)

In some rare cases a reverse proxy stops forwarding traffic to scm, before the response is returned to scm. In such a case the reverse proxy returns 502 (bad gateway), so we treat 502 not as error for restart actions. Co-authored-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
2025-11-08 06:25:45 +01:00 · 2022-02-02 10:02:46 +01:00
parent c155d1eb4a
commit dff5d3aa5b
5 changed files with 116 additions and 70 deletions
--- a/scm-ui/ui-api/src/apiclient.test.ts
+++ b/scm-ui/ui-api/src/apiclient.test.ts
@@ -24,7 +24,7 @@

 import { apiClient, createUrl, extractXsrfTokenFromCookie } from "./apiclient";
 import fetchMock from "fetch-mock";
-import { BackendError } from "./errors";
+import { BackendError, BadGatewayError } from "./errors";

 describe("create url", () => {
  it("should not change absolute urls", () => {
@@ -45,9 +45,9 @@ describe("error handling tests", () => {
    context: [
      {
        type: "planet",
-        id: "earth",
-      },
-    ],
+        id: "earth"
+      }
+    ]
  };

  afterEach(() => {
@@ -55,9 +55,9 @@ describe("error handling tests", () => {
    fetchMock.restore();
  });

-  it("should create a normal error, if the content type is not scmm-error", (done) => {
+  it("should create a normal error, if the content type is not scmm-error", done => {
    fetchMock.getOnce("/api/v2/error", {
-      status: 404,
+      status: 404
    });

    apiClient.get("/error").catch((err: Error) => {
@@ -67,13 +67,24 @@ describe("error handling tests", () => {
    });
  });

-  it("should create an backend error, if the content type is scmm-error", (done) => {
+  it("should create a bad gateway error", done => {
+    fetchMock.getOnce("/api/v2/error", {
+      status: 502
+    });
+
+    apiClient.get("/error").catch((err: Error) => {
+      expect(err).toBeInstanceOf(BadGatewayError);
+      done();
+    });
+  });
+
+  it("should create an backend error, if the content type is scmm-error", done => {
    fetchMock.getOnce("/api/v2/error", {
      status: 404,
      headers: {
-        "Content-Type": "application/vnd.scmm-error+json;v=2",
+        "Content-Type": "application/vnd.scmm-error+json;v=2"
      },
-      body: earthNotFoundError,
+      body: earthNotFoundError
    });

    apiClient.get("/error").catch((err: BackendError) => {
@@ -87,8 +98,8 @@ describe("error handling tests", () => {
      expect(err.context).toEqual([
        {
          type: "planet",
-          id: "earth",
-        },
+          id: "earth"
+        }
      ]);
      done();
    });
--- a/scm-ui/ui-api/src/apiclient.ts
+++ b/scm-ui/ui-api/src/apiclient.ts
@@ -25,12 +25,13 @@
 import { contextPath } from "./urls";
 import {
  BackendErrorContent,
+  BadGatewayError,
  createBackendError,
  ForbiddenError,
  isBackendError,
  TOKEN_EXPIRED_ERROR_CODE,
  TokenExpiredError,
-  UnauthorizedError,
+  UnauthorizedError
 } from "./errors";

 type SubscriptionEvent = {
@@ -62,7 +63,12 @@ type SubscriptionArgument = MessageListeners | SubscriptionContext;

 type Cancel = () => void;

-const sessionId = (Date.now().toString(36) + Math.random().toString(36).substr(2, 5)).toUpperCase();
+const sessionId = (
+  Date.now().toString(36) +
+  Math.random()
+    .toString(36)
+    .substr(2, 5)
+).toUpperCase();

 const extractXsrfTokenFromJwt = (jwt: string) => {
  const parts = jwt.split(".");
@@ -97,7 +103,7 @@ const createRequestHeaders = () => {
    // identify the web interface
    "X-SCM-Client": "WUI",
    // identify the window session
-    "X-SCM-Session-ID": sessionId,
+    "X-SCM-Session-ID": sessionId
  };

  const xsrf = extractXsrfToken();
@@ -107,10 +113,10 @@ const createRequestHeaders = () => {
  return headers;
 };

-const applyFetchOptions: (p: RequestInit) => RequestInit = (o) => {
+const applyFetchOptions: (p: RequestInit) => RequestInit = o => {
  if (o.headers) {
    o.headers = {
-      ...createRequestHeaders(),
+      ...createRequestHeaders()
    };
  } else {
    o.headers = createRequestHeaders();
@@ -134,6 +140,8 @@ function handleFailure(response: Response) {
      throw new UnauthorizedError("Unauthorized", 401);
    } else if (response.status === 403) {
      throw new ForbiddenError("Forbidden", 403);
+    } else if (response.status === 502) {
+      throw new BadGatewayError("Bad Gateway", 502);
    } else if (isBackendError(response)) {
      return response.json().then((content: BackendErrorContent) => {
        throw createBackendError(content, response.status);
@@ -169,7 +177,9 @@ class ApiClient {
  requestListeners: RequestListener[] = [];

  get = (url: string): Promise<Response> => {
-    return this.request(url, applyFetchOptions({})).then(handleFailure).catch(this.notifyAndRethrow);
+    return this.request(url, applyFetchOptions({}))
+      .then(handleFailure)
+      .catch(this.notifyAndRethrow);
  };

  post = (
@@ -196,7 +206,7 @@ class ApiClient {
    const options: RequestInit = {
      method: "POST",
      body: formData,
-      headers: additionalHeaders,
+      headers: additionalHeaders
    };
    return this.httpRequestWithBinaryBody(options, url);
  };
@@ -207,18 +217,22 @@ class ApiClient {

  head = (url: string) => {
    let options: RequestInit = {
-      method: "HEAD",
+      method: "HEAD"
    };
    options = applyFetchOptions(options);
-    return this.request(url, options).then(handleFailure).catch(this.notifyAndRethrow);
+    return this.request(url, options)
+      .then(handleFailure)
+      .catch(this.notifyAndRethrow);
  };

  delete = (url: string): Promise<Response> => {
    let options: RequestInit = {
-      method: "DELETE",
+      method: "DELETE"
    };
    options = applyFetchOptions(options);
-    return this.request(url, options).then(handleFailure).catch(this.notifyAndRethrow);
+    return this.request(url, options)
+      .then(handleFailure)
+      .catch(this.notifyAndRethrow);
  };

  httpRequestWithJSONBody = (
@@ -230,7 +244,7 @@ class ApiClient {
  ): Promise<Response> => {
    const options: RequestInit = {
      method: method,
-      headers: additionalHeaders,
+      headers: additionalHeaders
    };
    if (payload) {
      options.body = JSON.stringify(payload);
@@ -246,7 +260,7 @@ class ApiClient {
  ) => {
    const options: RequestInit = {
      method: method,
-      headers: additionalHeaders,
+      headers: additionalHeaders
    };
    options.body = payload;
    return this.httpRequestWithBinaryBody(options, url, "text/plain");
@@ -262,12 +276,14 @@ class ApiClient {
      options.headers["Content-Type"] = contentType;
    }

-    return this.request(url, options).then(handleFailure).catch(this.notifyAndRethrow);
+    return this.request(url, options)
+      .then(handleFailure)
+      .catch(this.notifyAndRethrow);
  };

  subscribe(url: string, argument: SubscriptionArgument): Cancel {
    const es = new EventSource(createUrlWithIdentifiers(url), {
-      withCredentials: true,
+      withCredentials: true
    });

    let listeners: MessageListeners;
@@ -308,11 +324,11 @@ class ApiClient {
  };

  private notifyRequestListeners = (url: string, options: RequestInit) => {
-    this.requestListeners.forEach((requestListener) => requestListener(url, options));
+    this.requestListeners.forEach(requestListener => requestListener(url, options));
  };

  private notifyAndRethrow = (error: Error): never => {
-    this.errorListeners.forEach((errorListener) => errorListener(error));
+    this.errorListeners.forEach(errorListener => errorListener(error));
    throw error;
  };
 }
--- a/scm-ui/ui-api/src/errors.ts
+++ b/scm-ui/ui-api/src/errors.ts
@@ -79,6 +79,15 @@ export class UnauthorizedError extends Error {
  }
 }

+export class BadGatewayError extends Error {
+  statusCode: number;
+
+  constructor(message: string, statusCode: number) {
+    super(message);
+    this.statusCode = statusCode;
+  }
+}
+
 export class TokenExpiredError extends UnauthorizedError {}

 export class ForbiddenError extends Error {
--- a/scm-ui/ui-api/src/plugins.ts
+++ b/scm-ui/ui-api/src/plugins.ts
@@ -27,6 +27,7 @@ import { isPluginCollection, PendingPlugins, Plugin, PluginCollection } from "@s
 import { useMutation, useQuery, useQueryClient } from "react-query";
 import { apiClient } from "./apiclient";
 import { requiredLink } from "./links";
+import { BadGatewayError } from "./errors";

 type WaitForRestartOptions = {
  initialDelay?: number;
@@ -37,31 +38,38 @@ const waitForRestartAfter = (
  promise: Promise<any>,
  { initialDelay = 1000, timeout = 500 }: WaitForRestartOptions = {}
 ): Promise<void> => {
-  const endTime = Number(new Date()) + 60000;
+  const endTime = Number(new Date()) + 4 * 60 * 1000;
  let started = false;

-  const executor =
-    <T = any>(data: T) =>
-    (resolve: (result: T) => void, reject: (error: Error) => void) => {
-      // we need some initial delay
-      if (!started) {
-        started = true;
-        setTimeout(executor(data), initialDelay, resolve, reject);
-      } else {
-        apiClient
-          .get("")
-          .then(() => resolve(data))
-          .catch(() => {
-            if (Number(new Date()) < endTime) {
-              setTimeout(executor(data), timeout, resolve, reject);
-            } else {
-              reject(new Error("timeout reached"));
-            }
-          });
-      }
-    };
+  const executor = <T = any>(data: T) => (resolve: (result: T) => void, reject: (error: Error) => void) => {
+    // we need some initial delay
+    if (!started) {
+      started = true;
+      setTimeout(executor(data), initialDelay, resolve, reject);
+    } else {
+      apiClient
+        .get("")
+        .then(() => resolve(data))
+        .catch(() => {
+          if (Number(new Date()) < endTime) {
+            setTimeout(executor(data), timeout, resolve, reject);
+          } else {
+            reject(new Error("timeout reached"));
+          }
+        });
+    }
+  };

-  return promise.then((data) => new Promise<void>(executor(data)));
+  return promise
+    .catch(err => {
+      if (err instanceof BadGatewayError) {
+        // in some rare cases the reverse proxy stops forwarding traffic to scm before the response is returned
+        // in such a case the reverse proxy returns 502 (bad gateway), so we treat 502 not as error
+        return "ok";
+      }
+      throw err;
+    })
+    .then(data => new Promise<void>(executor(data)));
 };

 export type UseAvailablePluginsOptions = {
@@ -72,10 +80,10 @@ export const useAvailablePlugins = ({ enabled }: UseAvailablePluginsOptions = {}
  const indexLink = useRequiredIndexLink("availablePlugins");
  return useQuery<PluginCollection, Error>(
    ["plugins", "available"],
-    () => apiClient.get(indexLink).then((response) => response.json()),
+    () => apiClient.get(indexLink).then(response => response.json()),
    {
      enabled,
-      retry: 3,
+      retry: 3
    }
  );
 };
@@ -88,10 +96,10 @@ export const useInstalledPlugins = ({ enabled }: UseInstalledPluginsOptions = {}
  const indexLink = useRequiredIndexLink("installedPlugins");
  return useQuery<PluginCollection, Error>(
    ["plugins", "installed"],
-    () => apiClient.get(indexLink).then((response) => response.json()),
+    () => apiClient.get(indexLink).then(response => response.json()),
    {
      enabled,
-      retry: 3,
+      retry: 3
    }
  );
 };
@@ -100,10 +108,10 @@ export const usePendingPlugins = (): ApiResult<PendingPlugins> => {
  const indexLink = useIndexLink("pendingPlugins");
  return useQuery<PendingPlugins, Error>(
    ["plugins", "pending"],
-    () => apiClient.get(indexLink!).then((response) => response.json()),
+    () => apiClient.get(indexLink!).then(response => response.json()),
    {
      enabled: !!indexLink,
-      retry: 3,
+      retry: 3
    }
  );
 };
@@ -135,19 +143,19 @@ export const useInstallPlugin = () => {
      return promise;
    },
    {
-      onSuccess: () => queryClient.invalidateQueries("plugins"),
+      onSuccess: () => queryClient.invalidateQueries("plugins")
    }
  );
  return {
    install: (plugin: Plugin, restartOptions: RestartOptions = {}) =>
      mutate({
        plugin,
-        restartOptions,
+        restartOptions
      }),
    isLoading,
    error,
    data,
-    isInstalled: !!data,
+    isInstalled: !!data
  };
 };

@@ -162,18 +170,18 @@ export const useUninstallPlugin = () => {
      return promise;
    },
    {
-      onSuccess: () => queryClient.invalidateQueries("plugins"),
+      onSuccess: () => queryClient.invalidateQueries("plugins")
    }
  );
  return {
    uninstall: (plugin: Plugin, restartOptions: RestartOptions = {}) =>
      mutate({
        plugin,
-        restartOptions,
+        restartOptions
      }),
    isLoading,
    error,
-    isUninstalled: !!data,
+    isUninstalled: !!data
  };
 };

@@ -196,18 +204,18 @@ export const useUpdatePlugins = () => {
      return promise;
    },
    {
-      onSuccess: () => queryClient.invalidateQueries("plugins"),
+      onSuccess: () => queryClient.invalidateQueries("plugins")
    }
  );
  return {
    update: (plugin: Plugin | PluginCollection, restartOptions: RestartOptions = {}) =>
      mutate({
        plugins: plugin,
-        restartOptions,
+        restartOptions
      }),
    isLoading,
    error,
-    isUpdated: !!data,
+    isUpdated: !!data
  };
 };

@@ -222,7 +230,7 @@ export const useExecutePendingPlugins = () => {
    ({ pending, restartOptions }) =>
      waitForRestartAfter(apiClient.post(requiredLink(pending, "execute")), restartOptions),
    {
-      onSuccess: () => queryClient.invalidateQueries("plugins"),
+      onSuccess: () => queryClient.invalidateQueries("plugins")
    }
  );
  return {
@@ -230,22 +238,22 @@ export const useExecutePendingPlugins = () => {
      mutate({ pending, restartOptions }),
    isLoading,
    error,
-    isExecuted: !!data,
+    isExecuted: !!data
  };
 };

 export const useCancelPendingPlugins = () => {
  const queryClient = useQueryClient();
  const { mutate, isLoading, error, data } = useMutation<unknown, Error, PendingPlugins>(
-    (pending) => apiClient.post(requiredLink(pending, "cancel")),
+    pending => apiClient.post(requiredLink(pending, "cancel")),
    {
-      onSuccess: () => queryClient.invalidateQueries("plugins"),
+      onSuccess: () => queryClient.invalidateQueries("plugins")
    }
  );
  return {
    update: (pending: PendingPlugins) => mutate(pending),
    isLoading,
    error,
-    isCancelled: !!data,
+    isCancelled: !!data
  };
 };