dougmoscrop / serverless-http

Use your existing middleware framework (e.g. Express, Koa) in AWS Lambda 🎉
Other
1.72k stars 166 forks source link

bug: If other JSON-related content type is used for responses instead of `application/json`, it throws an error #117

Closed diosney closed 5 years ago

diosney commented 5 years ago

I'm invoking directly a lambda that implements serverless-http, but for requests/responses uses https://jsonapi.org, so the content type is application/vnd.api+json, which is throwing an error:

(lines 22-26 create-requests.js file) https://github.com/dougmoscrop/serverless-http/blob/87dfe89ac0efaea56cf3f6df71e952cb17bd5b73/lib/provider/aws/create-request.js#L22

if (contentType && contentType.indexOf('application/json') === 0) {
      return Buffer.from(JSON.stringify(event.body));
    }

    throw new Error('event.body was an object but content-type is not json');

The expected behavior is that no matter what content type is used, if it is a JSON related one, it should pass correctly.

For that, I think that maybe searching for +json and /json besides application/json could do the trick, or to let the users to configure an array of expected content-types when defining the initial serverless() call, where the request and response transformations can be set.

dougmoscrop commented 5 years ago

I think you're right, but I think it can also be simplified further. If the body is an object, we don't need to touch the content type, we just need to stringify it anyway.

carmenates09 commented 5 years ago

@dougmoscrop Thank you for your quick response. I think the @diosney proposal makes sense, it may be the default as you say, but I think it cannot be restricted to a single 'Content Type', in the end both Content-type:application/vnd.api+json and application/json are json.

For example, in our project I am already using the content type application/vnd.api+json for the API and it works fine. But trying to call the same function but from an invoke is that we are presenting the problem.

Right now we are going to change all our answers for application/json and so we resolved, we just wanted to see your support to at least know that in the future we could return to using application/vnd.api+json.

Just know that we are grateful for your time.

diosney commented 5 years ago

@dougmoscrop Thanks!! That would be awesome, and makes sense too!

dougmoscrop commented 5 years ago

Can you try 2.1.1?

carmenates09 commented 5 years ago

@dougmoscrop Thanks +1 13 min you help us to fix it. Now is working fine and we continue using application/vnd.api+json thanks to you.

diosney commented 5 years ago

@dougmoscrop Wow! That was really fast, I didn't expected that, hahaha!

Thanks.