Merge pull request #315 from Reeywhaar/handle-error-http-code

Handle specific rest status codes in fetcher
2019-04-21 15:04:32 -05:00
parent 6b31741fa3 a37761fe9e
commit f4b0e0ca5f
4 changed files with 135 additions and 1 deletions
@@ -47,6 +47,9 @@ module.exports = {
        '@typescript-eslint/no-explicit-any': 0,
        '@typescript-eslint/no-object-literal-type-assertion': 0,
      },
+      globals: {
+        fail: true,
+      },
    },
    {
      files: ['*.test.ts', '*.test.tsx', '*.test.js', '*.test.jsx'],
@@ -0,0 +1,96 @@
+import fetcher from './fetcher';
+
+describe('fetcher', () => {
+  let originalHeaders = (window as any).Headers;
+
+  beforeAll(() => {
+    originalHeaders = (window as any).Headers;
+    (window as any).Headers = class {
+      append() {}
+      has() {
+        return false;
+      }
+      get() {
+        return null;
+      }
+    };
+  });
+
+  afterAll(() => {
+    (window as any).Headers = originalHeaders;
+  });
+
+  afterEach(() => {
+    (window.fetch as any).mockRestore();
+  });
+
+  describe('errors', () => {
+    it('should throw json on api json response with >= 400 status code', async () => {
+      const response = {
+        code: 2,
+        error: 'you just cant',
+        details: 'you just cant at all',
+      };
+      (window.fetch as any) = jest.fn().mockImplementation(async () => ({
+        status: 400,
+        headers: new (window as any).Headers(),
+        json: async () => response,
+        text: async () => JSON.stringify(response),
+      }));
+
+      return fetcher
+        .get('/api/some')
+        .then(data => {
+          fail(data);
+        })
+        .catch(e => {
+          expect(e.code).toBe(2);
+          expect(e.error).toBe('you just cant');
+          expect(e.details).toBe('you just cant at all');
+        });
+    });
+    it('should throw special error object on 401 status', async () => {
+      const response = '<html>unauthorized nginx response</html>';
+      (window.fetch as any) = jest.fn().mockImplementation(async () => ({
+        status: 401,
+        headers: new (window as any).Headers(),
+        json: async () => {
+          throw new Error('json parse error');
+        },
+        text: async () => response,
+      }));
+
+      return fetcher
+        .get('/api/some')
+        .then(data => {
+          fail(data);
+        })
+        .catch(e => {
+          expect(e.code).toBe(-1);
+          expect(e.error).toBe('Not authorized.');
+          expect(e.details).toBe('Not authorized.');
+        });
+    });
+    it('should throw "Something went wrong." object on unknown status', async () => {
+      (jest.spyOn(window, 'fetch') as any).mockImplementation(async () => ({
+        status: 400,
+        headers: new (window as any).Headers(),
+        json: async () => {
+          throw new Error('json parse error');
+        },
+        text: async () => 'you given me something wrong',
+      }));
+
+      return fetcher
+        .get({ url: '/api/some', logError: false })
+        .then(data => {
+          fail(data);
+        })
+        .catch(e => {
+          expect(e.code).toBe(-1);
+          expect(e.error).toBe('Something went wrong.');
+          expect(e.details).toBe('you given me something wrong');
+        });
+    });
+  });
+});
@@ -2,6 +2,7 @@ import { BASE_URL, API_BASE } from './constants';
 import { siteId } from './settings';
 import { StaticStore } from './static_store';
 import { getCookie } from './cookies';
+import { httpErrorMap } from '@app/utils/errorUtils';

 export type FetcherMethod = 'get' | 'post' | 'put' | 'patch' | 'delete' | 'head';
 const methods: FetcherMethod[] = ['get', 'post', 'put', 'patch', 'delete', 'head'];
@@ -79,6 +80,14 @@ const fetcher = methods.reduce<Partial<FetcherObject>>((acc, method) => {
      StaticStore.serverClientTimeDiff = timeDiff;

      if (res.status >= 400) {
+        if (httpErrorMap.has(res.status)) {
+          const errString = httpErrorMap.get(res.status)!;
+          throw {
+            code: -1,
+            error: errString,
+            details: errString,
+          };
+        }
        return res.text().then(text => {
          let err;
          try {
@@ -88,7 +97,11 @@ const fetcher = methods.reduce<Partial<FetcherObject>>((acc, method) => {
              // eslint-disable-next-line no-console
              console.error(err);
            }
-            throw 'Something went wrong.';
+            throw {
+              code: -1,
+              error: 'Something went wrong.',
+              details: text,
+            };
          }
          throw err;
        });
@@ -1,3 +1,7 @@
+/**
+ * map of codes that server returns in its response in case of error
+ * to client readable version
+ */
 const errorMessageForCodes = new Map([
  [0, 'Something went wrong. Please try again a bit later.'],
  [1, 'Comment cannot be found. Please refresh the page and try again.'],
@@ -20,9 +24,23 @@ const errorMessageForCodes = new Map([
  [18, 'Requested file cannot be found.'],
 ]);

+/**
+ * map of http rest codes to ui label, used by fetcher to generate error with `-1` code
+ */
+export const httpErrorMap = new Map([
+  [401, 'Not authorized.'],
+  [403, 'Forbidden.'],
+  [429, 'You have reached maximum request limit.'],
+]);
+
 export type FetcherError =
  | string
  | {
+      /**
+       * Error code, that is part of server error response.
+       *
+       * Note that -1 is reserved for error where `error` field shall be used directly
+       */
      code?: number;
      details?: string;
      error: string;
@@ -38,6 +56,10 @@ export function extractErrorMessageFromResponse(response: FetcherError): string
    return response;
  }

+  if (response.code === -1) {
+    return response.error;
+  }
+
  if (typeof response.code === 'number' && errorMessageForCodes.has(response.code)) {
    return errorMessageForCodes.get(response.code)!;
  }