spring-media / aws-lambda-router

Improved routing for AWS Lambda like SNS and ApiGateway
Apache License 2.0
98 stars 52 forks source link

ProxyIntegration/Typescript: Not returning ProxyIntegrationResult #58

Open TomiTakussaari opened 3 years ago

TomiTakussaari commented 3 years ago

Hi

Updated aws-lambda-router (from 0.6.2) on our project using Typescript, and noticed strange issue.

Aws-lambda-router prevents returning non-string responses from proxyIntegration actions here: https://github.com/spring-media/aws-lambda-router/blob/master/lib/proxyIntegration.ts#L22

So our old code, which returns plain JS objects, does not work anymore, as it fails Typescript checks.

However, if I JSON.stringify my response object, before returning it from action, aws-lambda-router wraps it with another JSON.stringify call here: https://github.com/spring-media/aws-lambda-router/blob/master/lib/proxyIntegration.ts#L61 (because of condition in line 57 does not handle plain string responses).

If I do the old way, and return plain JS object from my action (and cast it to string to make typescript happy) everything seems to work as it should.

So, is something wrong with proxyIntegration Action types? Or should I be returning instances of ProxyIntegrationResult from my actions nowadays?

EDIT:

Our old code (for aws-lambda-router 0.6.2) looked like this:

import * as router from 'aws-lambda-router';

export const handler = router.handler({
  proxyIntegration: {
    cors: false,
    routes: [

      {
        path: '/foobar',
        method: 'GET',
        action: () => ({ isFoobar: true }),
      },
    ],
  },
});

With new aws-lambda-router (0.9.1), this fails with Typescript (4.1.3) error:

foo.ts:11:23 - error TS2322: Type '{ isFoobar: boolean; }' is not assignable to type 'string | Promise<string> | ProxyIntegrationResult | Promise<ProxyIntegrationResult>'.
  Type '{ isFoobar: boolean; }' is missing the following properties from type 'Promise<ProxyIntegrationResult>': then, catch, [Symbol.toStringTag], finally

11         action: () => ({ isFoobar: true }),
                         ~~~~~~~~~~~~~~~~~~~

  ../../node_modules/aws-lambda-router/lib/proxyIntegration.d.ts:20:13
    20     action: (request: ProxyIntegrationEvent<unknown>, context: APIGatewayEventRequestContext) => ProxyIntegrationResult | Promise<ProxyIntegrationResult> | string | Promise<string>;
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    The expected type comes from the return type of this signature.

Found 1 error.

If I change it to like this:

import * as router from 'aws-lambda-router';

export const handler = router.handler({
  proxyIntegration: {
    cors: false,
    routes: [

      {
        path: '/foobar',
        method: 'GET',
        action: () => {
          return JSON.stringify({ isFoobar: true });
        },
      },
    ],
  },
});

It passes typescript compilation, but calls JSON.stringify second time for that string, adding second set of quotes.

I can make it work like this:

import * as router from 'aws-lambda-router';

export const handler = router.handler({
  proxyIntegration: {
    cors: false,
    routes: [

      {
        path: '/foobar',
        method: 'GET',
        action: () => {
          return { isFoobar: true } as unknown as string;
        },
      },
    ],
  },
});

But that kind of kills the point of using typescript.

connystrecker commented 3 years ago

Hi, I upgraded the aws-lambda-router in an old JS project and checked the responses with different action return values.

exports.handler = router.handler({
    proxyIntegration: {
        cors: true,
        routes: [
            {
                path: '/availability/list',
                method: 'POST',
                action: request => {
                    return {text: "object example"}
                }
            }
        ],
        debug: true
    }
});

For both 0.6.2 and 0.9.1 I've got the same responses.

returning: 'plain string "example"' result:

{ 
  statusCode: 200,
  headers: { ... },
  body: '"plain string \\"example\\""' 
}

returning: {text: "object example"} result:

{ 
  statusCode: 200,
  headers: { ... },
  body: '{"test":"object example"}' 
}

returning: JSON.stringify({text: "object example"}) result:

{ 
  statusCode: 200,
  headers: { ... },
  body: '"{\\"test\\":\\"object example\\"}"' 
}

Since version 7.0 the aws-lambda-router doesn't work with callbacks anymore. So in our project I had to change the lambda call. But that was the only thing to modify.

from:

req.handler(articleEvent, {}, (err, result)=> {
    if (err) {
        console.log('error', err);
    } else {
        console.log('result', result);
    }
});

to:

req.handler(articleEvent, {})
    .then(result => {
        console.log('result', result);
    } )
    .catch(err => {
        console.log('error', err);
    });

using Typescript to:

req.handler(listEvent, context)
    .then((res:any) => console.log('result', res))
    .catch((err:any) => console.log('error', err));

Can U send us a non-working code example we can test with?

TomiTakussaari commented 3 years ago

Updated description with code examples.

Problem of that second JSON.stringify call, when returning string from action, seems to actually be visible in your last message too:

  Returning 1:   JSON.stringify({text: "object example"})
  Returning 2: {text: "object example"}

  Actual Response1: body: '{"test":"object example"}' 
  Actual Response2: body: '"{\\"test\\":\\"object example\\"}"' 
connystrecker commented 3 years ago

Sorry, I've tested it in an old JS project. In old Typescript projects you can simply use

action: (request: any): any => ({isFoobar: true })

or

action: (request: any): ProxyIntegrationResult => ({body: JSON.stringify({isFoobar: true })})

In both cases you get the following response

{
  statusCode: 200,
  body: '{"isFoobar":true}',
  headers: { 'Content-Type': 'application/json' }
}
TomiTakussaari commented 3 years ago

Ok, so perhaps aws-lambda-router types could then be modified a bit, perhaps like this?

// https://github.com/spring-media/aws-lambda-router/blob/master/lib/proxyIntegration.ts

type Result = string | ProxyIntegrationResult | unknown;
export interface ProxyIntegrationRoute {
    path: string;
    method: string;
    action: (request: ProxyIntegrationEvent<unknown>, context: APIGatewayEventRequestContext) => Result | Promise<Result>;
}

Something like that would allow returning either somekind of object (that can be JSONified), ProxyIntegrationResult, or plain string, without using any, as using it kind of kills the point of typescript IMO.