fix: propagate http-errors as they are (#3922)

This PR aims to handle the increased log alarm volume seen by the SREs.

It appears that we get a large number of alarms because a client
disconnects early from the front-end API. These errors are then
converted into 500s because of missing handling. These errors appear to
be caused by the `http-errors` library in a dependency.

We also introduced a log line whenever we see errors now a while back,
and I don't think we need this logging (I was also the one who
introduced it).

The changes in this PR are specifically:
- When converting from arbitrary errors, give `BadRequestError` a 400
status code, not a 500.
- Add a dependency on `http-errors` (which is already a transitive
dependency because of the body parser) and use that to check whether an
error is an http-error.
- If an error is an http error, then propagate it to the user with the
status code and message.
- Remove warning logs when an error occurs. This was introduced to make
it easier to correlate an API error and the logs, but the system hasn't
been set up for that (yet?), so it's just noise now.
- When logging errors as errors, only do that if their status code would
be 500.

package.json

@@ -127,6 +127,7 @@
     "gravatar-url": "^3.1.0",
     "hash-sum": "^2.0.0",
     "helmet": "^6.0.0",
+    "http-errors": "^2.0.0",
     "ip": "^1.1.8",
     "joi": "^17.3.0",
     "js-sha256": "^0.9.0",

src/lib/error/unleash-error.ts

@@ -86,6 +86,8 @@ const statusCode = (errorName: string): number => {
             return 403;
         case 'AuthenticationRequired':
             return 401;
+        case 'BadRequestError': //thrown by express; do not remove
+            return 400;
         default:
             return 500;
     }

src/lib/routes/util.ts

@@ -3,6 +3,7 @@ import { Response } from 'express';
 import { Logger } from '../logger';
 import { UnleashError } from '../error/unleash-error';
 import { fromLegacyError } from '../error/from-legacy-error';
+import createError from 'http-errors';
 
 export const customJoi = joi.extend((j) => ({
     type: 'isUrlFriendly',
@@ -27,6 +28,17 @@ export const handleErrors: (
     logger: Logger,
     error: Error,
 ) => void = (res, logger, error) => {
+    if (createError.isHttpError(error)) {
+        return (
+            res
+                // @ts-expect-error http errors all have statuses, but there are no
+                // types provided
+                .status(error.status ?? 400)
+                .json({ message: error.message })
+                .end()
+        );
+    }
+
     const finalError =
         error instanceof UnleashError ? error : fromLegacyError(error);
 
@@ -40,15 +52,7 @@ export const handleErrors: (
         );
     }
 
-    logger.warn(
-        `Original error message: ${error.message}. Processed error message: "${
-            finalError.message
-        }" Error ID: "${finalError.id}". Full, serialized error: ${format(
-            finalError.toJSON(),
-        )}`,
-    );
-
-    if (['InternalError', 'UnknownError'].includes(finalError.name)) {
+    if (finalError.statusCode === 500) {
         logger.error(
             `Server failed executing request: ${format(error)}`,
             error,

yarn.lock

@@ -3851,7 +3851,7 @@ http-cache-semantics@^4.1.0, http-cache-semantics@^4.1.1:
   resolved "https://registry.yarnpkg.com/http-cache-semantics/-/http-cache-semantics-4.1.1.tgz#abe02fcb2985460bf0323be664436ec3476a6d5a"
   integrity sha512-er295DKPVsV82j5kw1Gjt+ADA/XYHsajl82cGNQG2eyoPkvgUhX+nDIyelzhIWbbsXP39EHcI6l5tYs2FYqYXQ==
 
-http-errors@2.0.0:
+http-errors@2.0.0, http-errors@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/http-errors/-/http-errors-2.0.0.tgz#b7774a1486ef73cf7667ac9ae0858c012c57b9d3"
   integrity sha512-FtwrG/euBzaEjYeRqOgly7G0qviiXoJWnvEH2Z1plBdXgbyjv34pHTSb9zoeHMyDy33+DWy5Wt9Wo+TURtOYSQ==