Improve handling of body parsing errors

[rowanmanning]

When an invalid request body is encountered in arc.http or arc.http.async, it results in a thrown error which is uncatchable by user code. This means that a client error (malformed body, likely 400) results in a server error (500).

Steps to reproduce

1. Create a new POST http endpoint in a new app (examples expect src/http/post-index). Install @architect/functions in the app. Update the HTTP handler to look like this:

   const arc = require('@architect/functions');
   
   exports.handler = arc.http.async(async request => {
     console.log(request.body);
     return {
       statusCode: 200,
       html: 'test page'
     };
   });

2. Start the application with arc sandbox

3. Run the following command in a terminal to send a POST request to the application with an invalid JSON body:

   curl -i -XPOST -H "content-type: application/json" -d 'invalid' http://localhost:3333/

4. You should see the response output something like this (reduced to relevant information):

   HTTP/1.1 500 Internal Server Error
   ...
   <p>Invalid request body encoding or invalid JSON</p>
   

Expected behavior

1. I think that the default response should have a 400 status code and messaging which explains that the given response body is invalid

2. Ideally as a user I should be able to handle that error myself. E.g. as JSON, or with the same HTML templates I use to handle other errors

Environment

I don't think these are necessarily relevant, but might be useful to know:

• OS: Mac OS Big Sur (11.5.2)
• Node.js: v14.17.0
• @architect/functions version: 4.0.0

Workaround

I found that I could work around this by defining my own wrapper around arc.http.async, but for me it'd be better if the framework handled this case. I'm also relying on the error message for this case remaining the same in Architect, which feels very brittle:

const arc = require('@architect/functions');

function createHttpHandler(...fns) {
  const arcHttpHandler = arc.http.async(...fns);
  return async (...args) => {
    let request;
    try {
      return await arcHttpHandler(...args);
    } catch (error) {

      if (/invalid (request body|json)/.test(error.message.toLowerCase())) {
        error = new Error('Please provide a valid JSON response body');
        error.statusCode = 400;
      }

      return {
        statusCode: error.statusCode || 500,
        json: {
          error: error.message
        }
      };
    }
  };
};

Possible solutions

Part 1: fixing the incorrect HTTP status code

It seems like there needs to be a try/catch around the body parsing here and here. In the catch we could shortcut by calling response with a representation of the error. This only solves the part where we're sending an incorrect error code, but wouldn't allow for easy customisation.

Part 2: allowing for this error to be handled by a user

I'm really not sure what the ideal way around this is. Possibly passing the error onto the next middleware somehow? If you have a direction you'd like to go in then I'd be comfortable trying to PR something.

Alternatives to a fix for this

An alternative (less ideal for me, but makes the workaround less brittle) would be to add code property to the error that's throw by bodyParser here. Something like:

const jsonError = new Error('Invalid request body encoding or invalid JSON');
jsonError.code = 'INVALID_REQUEST_BODY';
throw jsonError;