Pulumi: Function arity is not preserved in lambda artefact - breaks connect (therefore express and others)

[melbourne2991]

When you call connectInstance.use it accepts a standard handler and an error handler. The way that connect determined whether what has been passed in is an error handler or a regular handler is by looking at Function.length which returns the number of explicit arguments the function takes. You can see here:

https://github.com/senchalabs/connect/blob/master/index.js#L232-L241

Currently the way that pulumi appears to bundle functions transforms this:

function errorHandler(err, req, res, next) {
    const statusCode = err.statusCode;
    const message = err.message;
    res.status(statusCode).json(message);
};

into this

function __errorHandler() {
  return (function() {
    with({ errorHandler: __errorHandler }) {

return function /*errorHandler*/(err, req, res, next) {
    const statusCode = err.statusCode;
    const message = err.message;
    res.status(statusCode).json(message);
};

    }
  }).apply(undefined, undefined).apply(this, arguments);
}

__errorHandler is what is passed into connect's use method, and because it has zero explicit arguments (it uses the arguments object with apply instead) the function length is zero and therefore connect does not recognize this as an error handler which breaks the app.

[CyrusNajmabadi]

Can you show the caller of connectInstance.use? I have an idea on how it might be rewritten to support this. Thanks!

[lukehoban]

For reference - see all the examples at: https://github.com/senchalabs/connect

In particular examples like these: https://github.com/senchalabs/connect#error-middleware

[CyrusNajmabadi]

@lukehoban I'd be surprised if we hit this issue with that type of code. i.e. those examples are not actually capturing a function. My theory is that this is being hit more with:

app.use(onError);

function onError(err, req, res, next) {
}

Because this is a captured function, we'll try to serialize it.

My theory on how to workaround this (but i'll have to validate) would be to write things more like:

app.use((err, req, res, next) => onError(err, req, res, next));

function onError(err, req, res, next) {
}

Not pretty, but should hopefully address the issue by actually providing a func with the right amount of params taht sencha wants.

[melbourne2991]

@CyrusNajmabadi The caller looks like this:

// ... other imports
import { errorHandler }  from './error'

export function createApi() {
  const app = express();
  const parseCookies = cookieParser();

  app.disable("x-powered-by");

  app.get(<some handlers>);
  app.get(<some handlers>);

  app.use(errorHandler);

  return serverless(app);
}

createApi is then passed as a callbackFactory to new aws.lambda.CallbackFunction.

Also in terms of your work around I wasn't sure if you meant that to be a work around in the source code or in the serialized code - nevertheless I tried that in the source code and it does indeed seem to fix the problem! Does the serializer simply treat it differently because the function is inline as opposed to a reference?

[CyrusNajmabadi]

Also in terms of your work around I wasn't sure if you meant that to be a work around in the source code or in the serialized code

I meant it more as the former. i.e. as a workaround that you would be able to employ :)

nevertheless I tried that in the source code and it does indeed seem to fix the problem! Does the serializer simply treat it differently because the function is inline as opposed to a reference?

Precisely. When you have the actual function declaration there, then we don't touch it at all. The question is how we deal with references (i.e. the function decl is elsewhere, but we reference it by name). We need to do the strange sort of 'inverting' that we do to both make a referencable function, and make it so that other functino can properly capture the data it needs (which might be other functions, which might capture other stuff, et cetera, et cetera).

The problem is that we try to keep this 'rewriting' semantically invisible. But clearly you've run into a prime example of how we failed there (since the parameter lengths changed).

So, the workaround is to provide a declaration with the right parameter count (which we don't touch), and then forward the args to the function that we do rewrite.

--

I'm going to keep thsi open to track this issue. but i'm also going to treat this as lower pri since there's a workaround. Thanks!