api: Add new ApiError type and constructor-helper

Add a type to be used for API errors. Replace existing `makeApiError`
helper with the slightly-more-rigorous `makeErrorFromApi`.

In theory, one could make `ApiError` generic over `T: ApiResponseErrorData`,
and refine errors to their appropriate types at runtime. However, this
would be a lot of machinery (both type-level and runtime) for effectively
no present benefit: we don't have a use yet for the additional data on any
of the precise types.

Based loosely on a series of commits by Boris Yankov in PR #3484.

src/api/apiErrors.js

@@ -0,0 +1,45 @@
+/* @flow strict-local */
+import type { ApiErrorCode, ApiResponseErrorData } from './transportTypes';
+import * as logging from '../utils/logging';
+
+/** Runtime class of custom API error types. */
+export class ApiError extends Error {
+  code: ApiErrorCode;
+  data: $ReadOnly<{ ... }>;
+  httpStatus: number;
+
+  constructor(httpStatus: number, data: $ReadOnly<ApiResponseErrorData>) {
+    // eslint-disable-next-line no-unused-vars
+    const { result, code, msg, ...rest } = data;
+    super(msg);
+    this.data = rest;
+    this.code = code;
+    this.httpStatus = httpStatus;
+  }
+}
+
+/**
+ * Given a server response (allegedly) denoting an error, produce an Error to be
+ * thrown.
+ *
+ * If the `data` argument is actually of the form expected from the server, the
+ * returned error will be an {@link ApiError}; otherwise it will be a generic
+ * Error.
+ */
+export const makeErrorFromApi = (httpStatus: number, data: mixed): Error => {
+  // Validate `data`, and construct the resultant error object.
+  if (typeof data === 'object' && data !== null) {
+    if (data.result === 'error' && typeof data.msg === 'string') {
+      // If `code` is present, it must be a string.
+      if (!('code' in data) || typeof data.code === 'string') {
+        // Default to 'BAD_REQUEST' if `code` is not present.
+        return new ApiError(httpStatus, { code: 'BAD_REQUEST', ...data });
+      }
+    }
+  }
+
+  // Server has responded, but the response is not a valid error-object.
+  // (This should never happen, even on old versions of the Zulip server.)
+  logging.warn(`Bad response from server: ${JSON.stringify(data)}`);
+  return new Error('Server responded with invalid message');
+};

src/api/apiFetch.js

@@ -5,6 +5,7 @@ import { getAuthHeaders } from './transport';
 import { encodeParamsForUrl, isValidUrl } from '../utils/url';
 import userAgent from '../utils/userAgent';
 import { networkActivityStart, networkActivityStop } from '../utils/networkActivity';
+import { makeErrorFromApi } from './apiErrors';
 
 const apiVersion = 'api/v1';
 
@@ -51,19 +52,6 @@ export const fetchWithAuth = async (auth: Auth, url: string, params: {} = {}) =>
 export const apiFetch = async (auth: Auth, route: string, params: {} = {}) =>
   fetchWithAuth(auth, `${auth.realm}/${apiVersion}/${route}`, params);
 
-const makeApiError = (httpStatus: number, data: ?{}) => {
-  const error = new Error('API');
-  if (data) {
-    // $FlowFixMe
-    data.code = data.code ?? 'BAD_REQUEST';
-  }
-  // $FlowFixMe
-  error.data = data;
-  // $FlowFixMe
-  error.httpStatus = httpStatus;
-  return error;
-};
-
 export const apiCall = async (
   auth: Auth,
   route: string,
@@ -79,7 +67,7 @@ export const apiCall = async (
     }
     // eslint-disable-next-line no-console
     console.log({ route, params, httpStatus: response.status, json });
-    throw makeApiError(response.status, json);
+    throw makeErrorFromApi(response.status, json);
   } finally {
     networkActivityStop(isSilent);
   }

src/events/eventActions.js

@@ -10,6 +10,7 @@ import eventMiddleware from './eventMiddleware';
 import { tryGetAuth } from '../selectors';
 import actionCreator from '../actionCreator';
 import progressiveTimeout from '../utils/progressiveTimeout';
+import { ApiError } from '../api/apiErrors';
 
 /** Convert an `/events` response into a sequence of our Redux actions. */
 export const responseToActions = (
@@ -87,12 +88,10 @@ export const startEventPolling = (queueId: number, eventId: number) => async (
       // protection from inadvertent DDOS
       await progressiveTimeout();
 
-      if (e.message === 'API') {
-        if (e.data && e.data.code === 'BAD_EVENT_QUEUE_ID') {
-          // The event queue is too old or has been garbage collected.
-          dispatch(deadQueue());
-          break;
-        }
+      if (e instanceof ApiError && e.code === 'BAD_EVENT_QUEUE_ID') {
+        // The event queue is too old or has been garbage collected.
+        dispatch(deadQueue());
+        break;
       }
     }
   }