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

feat(lambda-at-edge, nextjs-component): allow handler input for custom handler code #649

Merged
merged 13 commits into from
Oct 20, 2020

Conversation

emhagman
Copy link
Contributor

@emhagman emhagman commented Oct 5, 2020

Allow custom Lambda handler to be set. See #648

Set the handler via inputs.handler and has lambda-at-edge copy someCustomFile.js file from this.nextConfigDir into the Lambda function folder by reading the first part of someCustomFile.handler and copying via Builder underneath:

new Builder(..., ..., { handler: 'someCustomFile.js' })

Useful for hooking into 3rd party tracing / error handling libraries such as:
https://docs.sentry.io/platforms/node/guides/aws-lambda/

For example:

example:
  component: "@sls-next/serverless-component@next"
  inputs:
    handler: someCustomFile.handler

{nextConfigDir}/someCustomFile.js

const Sentry = require("@sentry/serverless");
Sentry.init({ dsn: "https://5a167080eb1c4ecea2a82f38ea16e024@o149282.ingest.sentry.io/1196981" });

const lambdaAtEdge = require("./index"); // index.js file created by lambda-at-edge
exports.handler = Sentry.AWSLambda.wrapHandler(lambdaAtEdge.handler);

@emhagman emhagman changed the title feat(handler): allow handler input for custom handler WIP: allow handler input for custom handler Oct 5, 2020
@emhagman emhagman changed the title WIP: allow handler input for custom handler WIP feat(custom-handler): allow handler input for custom handler Oct 5, 2020
@emhagman emhagman changed the title WIP feat(custom-handler): allow handler input for custom handler feat(custom-handler): allow handler input for custom handler Oct 5, 2020
@emhagman
Copy link
Contributor Author

emhagman commented Oct 5, 2020

12.x build seems to be failing due to S3 error

@dphang
Copy link
Collaborator

dphang commented Oct 5, 2020

12.x build seems to be failing due to S3 error

Don't worry about that, it's because forks don't have access to AWS keys, I will check how we can detect this is a fork and skip this. I thought I had added a check but it seems it may not be working.

@emhagman
Copy link
Contributor Author

emhagman commented Oct 5, 2020

Sounds good! Ideally we could add an example custom handler somewhere in the docs but I wasn't sure where to put that or if I should start a new section.

const originalLambda = require("./index"); // index.js file created by lambda-at-edge

/**
 * Our customer handler
 * @param event
 * @param context
 * @returns {Promise<void>}
 */
exports.handler = async function(event, context) {
  console.log(`CUSTOM HANDLER BEING USED!`);
  context.callbackWaitsForEmptyEventLoop = false; // https://www.mongodb.com/blog/post/serverless-development-with-nodejs-aws-lambda-mongodb-atlas
  return originalLambda.handler(event, context); // call the original handler
}

@emhagman emhagman changed the title feat(custom-handler): allow handler input for custom handler feat(lambda-at-edge, nextjs-component): allow handler input for custom handler Oct 6, 2020
@emhagman emhagman changed the title feat(lambda-at-edge, nextjs-component): allow handler input for custom handler feat(lambda-at-edge, nextjs-component): allow handler input for custom handler code Oct 6, 2020
@dphang
Copy link
Collaborator

dphang commented Oct 6, 2020

Sounds good! Ideally we could add an example custom handler somewhere in the docs but I wasn't sure where to put that or if I should start a new section.

const originalLambda = require("./index"); // index.js file created by lambda-at-edge

/**
 * Our customer handler
 * @param event
 * @param context
 * @returns {Promise<void>}
 */
exports.handler = async function(event, context) {
  console.log(`CUSTOM HANDLER BEING USED!`);
  context.callbackWaitsForEmptyEventLoop = false; // https://www.mongodb.com/blog/post/serverless-development-with-nodejs-aws-lambda-mongodb-atlas
  return originalLambda.handler(event, context); // call the original handler
}

Maybe start a new section? Also does this override both API and default handlers?

@emhagman
Copy link
Contributor Author

emhagman commented Oct 6, 2020

@dphang Yes, it overrides both handlers. I could separate it out if desired

@dphang
Copy link
Collaborator

dphang commented Oct 6, 2020

@dphang Yes, it overrides both handlers. I could separate it out if desired

I think it's ok now, any thoughts @danielcondemarin?

@danielcondemarin
Copy link
Contributor

@dphang Yes, it overrides both handlers. I could separate it out if desired

I think it's ok now, any thoughts @danielcondemarin?

Probably best to make it consistent with the other lambda inputs like memory, name etc. See readLambdaInputValue

@kulesa
Copy link

kulesa commented Oct 16, 2020

@danielcondemarin a custom handler probably doesn't need lambda inputs because it does not add a new lambda function. It is only a wrapper around the default lambda and it gets bundled and deployed with the default lambda handler.

@emhagman @dphang can I contribute to this pull request in any way to get it merged? We're about to start using the fork of the component with custom handler in production 😄 , so I really hope this feature makes it to the library eventually.

@dphang
Copy link
Collaborator

dphang commented Oct 16, 2020

Yeah if you fix the conflicts I think I'm fine with it

@emhagman
Copy link
Contributor Author

emhagman commented Oct 20, 2020

I fixed the conflicts. One thing to note is that now that we have minifyHandlers as an option, this custom handler does not follow that option since it appears that all that flag does is copy the .min.js version of the default handlers over if enabled.

We should either say somewhere in the docs that custom handlers aren't minified or pull in a tool to minify them separately.

@dphang Thoughts?

@codecov
Copy link

codecov bot commented Oct 20, 2020

Codecov Report

Merging #649 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #649      +/-   ##
==========================================
+ Coverage   80.09%   80.15%   +0.05%     
==========================================
  Files          54       54              
  Lines        1718     1723       +5     
  Branches      358      363       +5     
==========================================
+ Hits         1376     1381       +5     
  Misses        284      284              
  Partials       58       58              
Impacted Files Coverage Δ
packages/libs/lambda-at-edge/src/build.ts 94.03% <100.00%> (+0.05%) ⬆️
...rless-components/nextjs-component/src/component.ts 90.14% <100.00%> (+0.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74a8397...d691b65. Read the comment docs.

@dphang
Copy link
Collaborator

dphang commented Oct 20, 2020

I fixed the conflicts. One thing to note is that now that we have minifyHandlers as an option, this custom handler does not follow that option since it appears that all that flag does is copy the .min.js version of the default handlers over if enabled.

We should either say somewhere in the docs that custom handlers aren't minified or pull in a tool to minify them separately.

@dphang Thoughts?

Yeah, I think we should just update the docs for that (you can do in another PR or I can update them). If you want, you can add Terser in another PR to minify custom handlers.

I did think about minifying at build time but it makes more sense to minify in the distributed component since it does add a few seconds to build time. I figure most people won't care about how it's minified, so I had provided a sensible default set of Terser options.

@dphang
Copy link
Collaborator

dphang commented Oct 20, 2020

@dphang dphang merged commit cecd327 into serverless-nextjs:master Oct 20, 2020
@emhagman
Copy link
Contributor Author

Sounds good! I can open another PR for the docs.

Every year I do https://hacktoberfest.digitalocean.com as part of a Fall tradition. Do you mind adding hacktoberfest-accepted as a label to this PR if you think this PR was helpful to the project so I can have it counted.

@mattbbc
Copy link

mattbbc commented Jun 8, 2021

Hi @emhagman, were the docs updated to show this off? I would like to write a custom handler to redirect users to a certain endpoint determined by CloudFront geolocation headers and this seems like a good fit.

@ekasprzyk
Copy link

Hello! @emhagman or @dphang
I managed to get a custom handler working for logging, but there's a critical bit I'm missing around imports and I was wondering if either of you could help? (As there really are no docs for this anywhere else)

I have the following code that works in deploying a handler:

const lambdaAtEdge = require('../index');

const handler = async (event, context) => {
  console.log('Pre-handler');
  const result = lambdaAtEdge.handler(event, context);
  console.log('Post-handler');

  return result;
};

exports.handler = handler;

I put the handler in the /src directory which is why the .. exists on the import.

I'm trying to add in a Lumigo tracer, which seems similar to Sentry. In your example you just require it, so I've tried the same:

const lumigo = require('@lumigo/tracer')({ token: 'REDACTED' });
const lambdaAtEdge = require('../index');

const handler = async (event, context) => {
  console.log('Pre-handler');
  const result = lambdaAtEdge.handler(event, context);
  console.log('Post-handler');

  return result;
};

exports.handler = lumigo.trace(handler);

and I'm not having any joy. It fails with:

Uncaught Exception {"errorType":"Runtime.ImportModuleError","errorMessage":"Error: Cannot find module '@lumigo/tracer'

What am I missing? I notice the example links to Sentry's docs which has a webpack config entry attached to it, but I don't have any similar thing to use from Lumigo.

Thanks in advance.

@ekasprzyk
Copy link

Just to say, I figured out a solution. For anyone else having the same issues as I am and who's not hugely hot on Serverless/NextJS internals or done a lot of webpack config (as is the case with me):

In order to use a wrapper (I used Lumigo) you need to set the handler as documented here, but the code you run is bundled elsewhere and separate from the lambda you're attempting to run, so adding your dependency as a project dependency isn't going to work.

What you have to do is actually:

  1. Add the handler
  2. Bundle the handler's dependencies separately with webpack
  3. Add the bundle to your lambdas for upload to the cloud service provider
  4. Include the bundle in your code

e.g. If I want to do:

const lambdaAtEdge = require('../index');
const lumigo = require('@/lumigo/tracer')({token: token});

const handler = async (event, context) => {
  console.log('Pre-handler');
  const result = lambdaAtEdge.handler(event, context);
  console.log('Post-handler');

  return result;
};

exports.handler = lumigo.trace(handler);

then what I have to do is actually:

const lambdaAtEdge = require('../index');
const wrapWithLumigo = require('../lumigo-wrapper');

const handler = async (event, context) => {
  console.log('Pre-handler');
  const result = lambdaAtEdge.handler(event, context);
  console.log('Post-handler');

  return result;
};

exports.handler = wrapWithLumigo.wrapWithLumigo(handler);
const path = require('path');

module.exports = {
  entry: './handler.js',
  output: {
    path: path.resolve(__dirname, 'handler_dist'),
    filename: 'lumigo-wrapper.js',
    library: {
      name: 'lumigo-wrapper',
      type: 'umd',
    },
    globalObject: 'this',
  },
};

which refers to:

const lumigo = require('@lumigo/tracer')({ token: secret_token});

export function  wrapWithLumigo (handler) {
  return lumigo.trace(handler);
}

Copy the bundle and deploy using the postBuild script:
#767 (comment)

e.g.

const fs = require('fs-extra');

console.log('-> Copying lumigo tracer...');
const wrapperSrc = './serverless_files';
const wrapperDefaultDest = './.serverless_nextjs/default-lambda';
const wrapperApiDest = './.serverless_nextjs/api-lambda';
fs.copySync(wrapperSrc, wrapperDefaultDest, { recursive: true });
fs.copySync(wrapperSrc, wrapperApiDest, { recursive: true });
console.log('Lumigo tracer was copied successfully');

(This assumes I manually copied the result of the dist bundle into serverless_files)

The bundle generated in 2. is deployed in 3. and picked up in the handler in 1.

Hope this helps.

@J3tto
Copy link
Contributor

J3tto commented Mar 25, 2022

@ekasprzyk Did you ever get Lumigo Tracing working on the api-lambda.

Using the process you suggest the other all work but get Cannot read property 'headers' of undefined on the api-lambda.

It seems to occur after the original handler completes so it doesn't look to be an issues on the serverless-nextjs side.

@ekasprzyk
Copy link

@J3tto Yes we did, and started to expand our Lumigo tracing into another application, where it worked (except for a bug supposedly on the Lumigo end causing stats not to be reported, which is apparently fixed).

We are using next/serverless versions slightly behind the current latest (10/1.x) so that might contribute.

The fact that it's happening after the original handler returns is weird.

@HemeanandhB
Copy link

I am trying to use custom handler in my application to forward Cloudfront headers back to browser. I have the following code in serverless.json

"inputs": { "handler": "handler.handler",............}.

Custom handler gets added only for default & Api lambdas. The logic written in handler works. But ISR stopped working from then. I could see the following error for Regeneration lambda in cloudwatch. This gets logged frequently after adding handler:
{
"errorType": "Runtime.ImportModuleError",
"errorMessage": "Error: Cannot find module 'handler'\nRequire stack:\n- /var/runtime/index.mjs",
"stack": [
"Runtime.ImportModuleError: Error: Cannot find module 'handler'",
"Require stack:",
"- /var/runtime/index.mjs",
" at _loadUserApp (file:///var/runtime/index.mjs:951:17)",
" at async Object.UserFunction.js.module.exports.load (file:///var/runtime/index.mjs:976:21)",
" at async start (file:///var/runtime/index.mjs:1137:23)",
" at async file:///var/runtime/index.mjs:1143:1"
]
}

This makes ISR to fail which is concern for my application. Is there an option to add handlers only for default lambda alone. or could we add handlers with out affecting Incremental Static Regeneration
@emhagman @dphang
Please advice

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

Successfully merging this pull request may close these issues.

None yet

8 participants