Simplify error handling within the function processRequest.

Author: jaydenseric

changelog.md

@@ -6,14 +6,14 @@
 
 - Updated the [`busboy`](https://npm.im/busboy) dependency to v1, fixing [#311](https://github.com/jaydenseric/graphql-upload/issues/311).
   - This important update addresses the vulnerability [CVE-2022-24434](https://nvd.nist.gov/vuln/detail/CVE-2022-24434) ([GHSA-wm7h-9275-46v2](https://github.com/advisories/GHSA-wm7h-9275-46v2)).
-  - It now emits an error on a file upload stream if the request disconnects partway through, which is different to the error that used to come from [`graphql-upload`](https://npm.im/graphql-upload).
   - Some error messages have changed.
   - Temporarily until [mscdex/busboy#297](https://github.com/mscdex/busboy/issues/297) is fixed upstream, for the function `processRequest` and the middleware `graphqlUploadExpress` and `graphqlUploadKoa` the option `maxFileSize` is actually 1 byte less than the amount specified.
 
 ### Patch
 
 - Updated the [`typescript`](https://npm.im/typescript) dev dependency.
 - In the function `processRequest` use the `on` method instead of `once` to listen for `error` events on the [`busboy`](https://npm.im/busboy) parser, as in edge cases the same parser could have multiple `error` events and all must be handled to prevent the Node.js process exiting with an error.
+- Simplified error handling within the function `processRequest`.
 - Added a test for the function `processRequest` with a maliciously malformed multipart request.
 
 ## 14.0.0

processRequest.js

@@ -42,9 +42,6 @@ function processRequest(
     /** @type {Error} */
     let exitError;
 
-    /** @type {import("stream").Readable} */
-    let lastFileStream;
-
     /**
      * @type {{ [key: string]: unknown } | Array<
      *   { [key: string]: unknown }
@@ -76,27 +73,20 @@ function processRequest(
     /**
      * Exits request processing with an error. Successive calls have no effect.
      * @param {Error} error Error instance.
+     * @param {boolean} [isParserError] Is the error from the parser.
      */
-    const exit = (error) => {
+    function exit(error, isParserError = false) {
       if (exitError) return;
 
       exitError = error;
 
-      reject(exitError);
-
-      parser.destroy();
-
-      if (
-        lastFileStream &&
-        !lastFileStream.readableEnded &&
-        !lastFileStream.destroyed
-      )
-        lastFileStream.destroy(exitError);
-
       if (map)
         for (const upload of map.values())
           if (!upload.file) upload.reject(exitError);
 
+      // If the error came from the parser, don’t cause it to be emitted again.
+      isParserError ? parser.destroy() : parser.destroy(exitError);
+
       request.unpipe(parser);
 
       // With a sufficiently large request body, subsequent events in the same
@@ -106,7 +96,9 @@ function processRequest(
       setImmediate(() => {
         request.resume();
       });
-    };
+
+      reject(exitError);
+    }
 
     parser.on("field", (fieldName, value, { valueTruncated }) => {
       if (valueTruncated)
@@ -228,8 +220,6 @@ function processRequest(
     parser.on(
       "file",
       (fieldName, stream, { filename, encoding, mimeType: mimetype }) => {
-        lastFileStream = stream;
-
         if (!map) {
           ignoreStream(stream);
           return exit(
@@ -331,7 +321,9 @@ function processRequest(
     // could have multiple `error` events and all must be handled to prevent the
     // Node.js process exiting with an error. One edge case is if there is a
     // malformed part header as well as an unexpected end of the form.
-    parser.on("error", exit);
+    parser.on("error", (/** @type {Error} */ error) => {
+      exit(error, true);
+    });
 
     response.once("close", () => {
       released = true;

processRequest.test.mjs

@@ -789,8 +789,11 @@ export default (tests) => {
                 stream.once("error", reject).once("end", resolve).resume();
               }),
               {
-                name: "Error",
-                message: "Unexpected end of file",
+                name: "BadRequestError",
+                message:
+                  "Request disconnected during file upload stream parsing.",
+                status: 499,
+                expose: true,
               }
             );
           };
@@ -931,8 +934,11 @@ export default (tests) => {
             strictEqual(upload.mimetype, "text/plain");
             strictEqual(upload.encoding, "7bit");
             throws(() => upload.createReadStream(), {
-              name: "Error",
-              message: "Unexpected end of file",
+              name: "BadRequestError",
+              message:
+                "Request disconnected during file upload stream parsing.",
+              status: 499,
+              expose: true,
             });
           };