Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow importing handler from layers #20

Open
ewindisch opened this issue Jun 11, 2019 · 15 comments
Open

Allow importing handler from layers #20

ewindisch opened this issue Jun 11, 2019 · 15 comments

Comments

@ewindisch
Copy link

It would be useful to be able to load handler modules from layers! (I do this with other runtimes)

Options would be to include /opt/nodejs/node_modules in the search path, or to allow absolute imports.

@mhart
Copy link
Member

mhart commented Jun 11, 2019

This should work already – are you having problems?

@ewindisch
Copy link
Author

Correct. The import seems to look in the "LAMBDA_TASK_ROOT" for the handler.

$ serverless invoke -f hello -l
{
    "errorType": "Error",
    "errorMessage": "Cannot find module '/var/task//opt/nodejs/node_modules/@iopipe/iopipe'",
    "stackTrace": [
        "    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)",
        "    at Function.Module._load (internal/modules/cjs/loader.js:562:25)",
        "    at Module.require (internal/modules/cjs/loader.js:690:17)",
        "    at require (internal/modules/cjs/helpers.js:25:18)",
        "    at getHandler (/opt/bootstrap.js:148:15)",
        "    at start (/opt/bootstrap.js:25:15)",
        "    at Object.<anonymous> (/opt/bootstrap.js:20:1)",
        "    at Module._compile (internal/modules/cjs/loader.js:776:30)",
        "    at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)",
        "    at Module.load (internal/modules/cjs/loader.js:653:32)"
    ]
}

@ewindisch
Copy link
Author

ewindisch commented Jun 11, 2019

same error if I use @iopipe/iopipe.handler

{
    "errorType": "Error",
    "errorMessage": "Cannot find module '/var/task/@iopipe/iopipe'",
    "stackTrace": [
        "    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)",
        "    at Function.Module._load (internal/modules/cjs/loader.js:562:25)",
        "    at Module.require (internal/modules/cjs/loader.js:690:17)",
        "    at require (internal/modules/cjs/helpers.js:25:18)",
        "    at getHandler (/opt/bootstrap.js:148:15)",
        "    at start (/opt/bootstrap.js:25:15)",
        "    at Object.<anonymous> (/opt/bootstrap.js:20:1)",
        "    at Module._compile (internal/modules/cjs/loader.js:776:30)",
        "    at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)",
        "    at Module.load (internal/modules/cjs/loader.js:653:32)"
    ]
}

@mhart
Copy link
Member

mhart commented Jun 11, 2019

Oh sorry – I thought you were talking about NODE_PATH

Can you outline the use case again?

@ewindisch
Copy link
Author

@mhart I have a layer that contains my handler. In my personal use-case, that handler is a wrapper that then does an import and load of another handler... but it's also completely valid for users to have their entire application placed within layers.

Example serverless.yml:

service: aws-nodejs-lambci

provider:
  name: aws
  runtime: provided
  layers:
    - arn:aws:lambda:us-east-1:553035198032:layer:nodejs10:14
    - arn:aws:lambda:us-east-1:146318645305:layer:IOpipeNodeJS810:9

functions:
  hello:
    handler: '@iopipe/iopipe.handler' #'/opt/nodejs/node_modules/@iopipe/iopipe.handler'
    environment:
      IOPIPE_HANDLER: handler.hello

@mhart
Copy link
Member

mhart commented Jun 11, 2019

Oh, this works with existing AWS runtimes? I thought the handler always tried to resolve from LAMBDA_TASK_ROOT

@ewindisch
Copy link
Author

@mhart The official AWS ones do not for NodeJS, but this is confirmed to work with the official Java and Python runtimes from AWS . I've been nudging AWS to get this working in Node as well... but also, more immediately, seeing if we can get the popular custom Node runtimes to support this (it is already supported in NSolid by Nodesource).

@mhart
Copy link
Member

mhart commented Jun 11, 2019

Hmmm. I'm a little wary of diverging from AWS too much. I could see how absolute paths could be supported, but how do you distinguish between a module called index and a file called index.js?

Looking through the AWS Node.js 10 code, it looks like it might support ../../opt/nodejs/node_modules/@iopipe/iopipe.handler (but 8.10 and 6.10 won't) – I might consider modifying the logic to allow this too if that suits?

@ewindisch
Copy link
Author

My testing has been unsuccessful in using path traversals via '..' to enable this /w the official runtime.

Lambda has always supported importing from node_modules, this would ideally just allow importing from /opt/nodejs/node_modules... using the absolute path would be a solution here to avoid unintended consequences; this is already a requirement of the NSolid runtime and would help reduce drift between runtimes.

Simply allowing any import via an absolute path (or restricted to the task root and paths beginning with /opt) would suffice for me, and wouldn't cause any breakages moving to this runtime.

@mhart
Copy link
Member

mhart commented Jun 11, 2019

Interesting, this works for me:

mkdir opt
cd opt
npm install @iopipe/iopipe
cd ..
docker run -v $PWD/opt:/opt lambci/lambda:nodejs10.x ../../opt/node_modules/@iopipe/iopipe.handler

@mhart
Copy link
Member

mhart commented Jun 11, 2019

I haven't updated lambci/lambda:nodejs10.x in a couple of weeks, they might've changed the code to disallow this? But I would assume that would work fine on the nodejs10.x runtime. It currently outputs:

{
  "errorType": "Error",
  "errorMessage": "No IOPIPE_HANDLER environment variable set."
}

@ewindisch
Copy link
Author

ewindisch commented Jun 11, 2019 via email

@ewindisch
Copy link
Author

ewindisch commented Jun 17, 2019

Playing with this more, it seems that the official nodejs10.x runtime now supports the absolute path, but with lambci, I get Cannot find module '/var/task//opt/nodejs/node_modules/@iopipe/iopipe. Using '../../' didn't work and if it did, it would differ from AWS which errors if I use the .. path traversal)

@mhart
Copy link
Member

mhart commented Jun 17, 2019 via email

@mhart
Copy link
Member

mhart commented Jun 17, 2019

Oh wow, yeah they've changed a lot of things since the last time I checked. Mostly good! Although it's interesting they've removed the ability to do relative requires. They now throw Use absolute paths when specifying root directories in handler names if you try to to relative paths.

I'll update this runtime accordingly (and lambci/lambda:nodejs10.x)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants