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

Async handlers produce unhandled promise rejection warnings #43

Closed
lloydh opened this issue Jun 1, 2018 · 24 comments
Closed

Async handlers produce unhandled promise rejection warnings #43

lloydh opened this issue Jun 1, 2018 · 24 comments

Comments

@lloydh
Copy link

lloydh commented Jun 1, 2018

Test Repo

I'm getting warnings about unhandled promise rejection when using async handler functions like below.

exports.handler = async (event, context, callback) => {
  callback(null, {
    statusCode: 200,
    body: JSON.stringify({ success: true }),
  });
}

I've also tried using Promise.Resolve() with the response object.

exports.handler = async (event, context, callback) => {
  Promise.resolve({
    statusCode: 200,
    body: JSON.stringify({ success: true }),
  });
}

My understanding is that either should work with node 8.10.x lambda instances but the netlify-lambda serve console produces the following warning:

UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'statusCode' of undefined
    at callback (/Users/lloyd/dev/netlify-lambda-async-test/node_modules/netlify-lambda/lib/serve.js:22:42)
    at /Users/lloyd/dev/netlify-lambda-async-test/node_modules/netlify-lambda/lib/serve.js:41:21
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:182:7)
(node:56861) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
@mmiller42
Copy link

When an async function throws an error, the function doesn't throw the error on its invocation, it rejects the promise it returns. However, AWS uses callbacks, not promises, so it doesn't read the return value of your function (which is the rejected promise).

You should try something like this:

exports.handler = async (event, context, callback) => {
  try {
    // your logic here
   callback(null, result);
  } catch (err) {
    callback(err);
  }
};

Now you're explicitly passing any error that occurs to the callback, where Netlify/Lambda can actually read the error.

Your second example won't work, as Lambda doesn't use promises -- also, you have to make sure to return a promise from a function if you want it to be handled (but again, Lambda won't).

@i8ramin
Copy link

i8ramin commented Oct 14, 2018

I am also experiencing the same thing with my async callback. Even with the try/catch block. I put a console.log where the error is happening, and it looks like there are 2 requests happening for some reason. The first one succeeds, and the second one has an undefined lambdaResponse

Request from 108.27.71.64: GET /pr-merge

************** lambdaResponse { statusCode: 200, body: '' }
Response with status 200 in 472 ms.

************** lambdaResponse undefined
(node:3211) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'statusCode' of undefined
    at callback (/app/node_modules/netlify-lambda/lib/serve.js:24:42)
    at /app/node_modules/netlify-lambda/lib/serve.js:43:21
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:160:7)
(node:3211) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:3211) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

I have all my async/await functions surrounded by try/catch blocks and call callback(err) in the catch block.

@swyxio
Copy link
Contributor

swyxio commented Oct 17, 2018

thanks @mmiller42, i actually didnt know that :)

hi @i8ramin - gonna close this as seems more to do with AWS lambda syntax than this netlify-lambda utility. hope you figure it out.

@swyxio swyxio closed this as completed Oct 17, 2018
@nunoarruda
Copy link

@sw-yx can you reopen this? I'm having the same issue as @lloydh and @i8ramin but @mmiller42 proposed solution does not solve the problem.

Here's my demo:

import fetch from "node-fetch";

exports.handler = async function(event, context, callback) {
    try {
        const response = await fetch("https://api.chucknorris.io/jokes/random");
        const data = await response.json();

        callback(null, {
            statusCode: 200,
            body: data.value
        });
    } catch (err) {
        console.error(err);
    }
};

Log:

netlify-lambda: Starting server
Lambda server is listening on 9000
Hash: 533f41e1d4248894ae20
Version: webpack 4.26.1
Time: 966ms
Built at: 11/28/2018 10:59:44 PM
  Asset      Size  Chunks             Chunk Names
test.js  18.3 KiB       0  [emitted]  test
Entrypoint test = test.js
[0] external "stream" 42 bytes {0} [built]
[1] external "zlib" 42 bytes {0} [built]
[2] external "url" 42 bytes {0} [built]
[3] external "http" 42 bytes {0} [built]
[4] external "https" 42 bytes {0} [built]
[5] ./test.js + 1 modules 40.8 KiB {0} [built]
    | ./test.js 1.14 KiB [built]
    | ../node_modules/node-fetch/lib/index.mjs 39.6 KiB [built]
Request from ::1: GET /test
Response with status 200 in 685 ms.
(node:99167) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'statusCode' of undefined
    at callback (/Users/nunoarruda/Desktop/test/node_modules/netlify-lambda/lib/serve.js:22:42)
    at /Users/nunoarruda/Desktop/test/node_modules/netlify-lambda/lib/serve.js:41:21
    at process.internalTickCallback (internal/process/next_tick.js:77:7)
(node:99167) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:99167) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@swyxio swyxio reopened this Dec 3, 2018
@8eecf0d2
Copy link
Contributor

8eecf0d2 commented Dec 4, 2018

From the examples and logs I'm guessing the promise rejection is within netlify-lambda, and specifically the logic behind determining if a response / callback is a Promise or not.

Will have a go at fixing tonight.

@talves
Copy link
Contributor

talves commented Dec 5, 2018

@nunoarruda at first I was seeing the same error, but can't reproduce it, so my assumptions were incorrect. Sorry for the noise.

@talves
Copy link
Contributor

talves commented Dec 5, 2018

With the new Node.js 8.10 runtime, there are new handler types that can be declared with the “async” keyword or can return a promise directly.

Using async/await

import fetch from "node-fetch";

exports.handler = async function(event, context) {
    try {
        const response = await fetch("https://api.chucknorris.io/jokes/random");
        if (!response.ok) { // NOT res.status >= 200 && res.status < 300
            return { statusCode: response.status, body: response.statusText };
        }
        const data = await response.json();

        return {
            statusCode: 200,
            body: data.value
            // if you want to return whole json string
            // headers: { 'Content-Type': 'application/json' },
            // body: JSON.stringify(data)
        };
    } catch (err) {
        console.log(err); // output to netlify function log
        return {
            statusCode: 500,
            body: err.message // Could be a custom message or object i.e. JSON.stringify(err)
        };
    }
};

Returning a promise

import fetch from "node-fetch";

exports.handler = (event, context) => {
  return new Promise((resolve, reject) => {
    fetch('https://api.chucknorris.io/jokes/random')
    .then(res => {
      if (res.ok) { // res.status >= 200 && res.status < 300
        return res.json();
      } else {
        resolve({ statusCode: res.status, body: res.statusText })
      };
    })
    .then(data =>{
      const response = {
        statusCode: 200,
        headers: { 'content-type': 'application/json' },
        body: JSON.stringify(data)
      }
      resolve(response);
    })
    .catch(err => {
      console.log(err)
      resolve({ statusCode: 500, body: err.message });
    })
  });
};

I am not an AWS lambda expert, but this is the way I interpret using these new handlers in v8.10
Please let me know if this is the way we should be using these with Netlify functions.
BTW, these are both working for me in a site on Netlify.

@talves
Copy link
Contributor

talves commented Dec 6, 2018

@sw-yx Shawn, I don't think we should be using callback with the two handler types above in the previous comment, but I was hoping someone at Netlify could confirm this for me.

@swyxio
Copy link
Contributor

swyxio commented Dec 6, 2018

i think you are correct Tony. the docs indicate the same thing. Here's Serverless calling it out as a mistake as well

i'm sadly far from a lambda expert. just trying to help maintain things as an employee with commit rights and a heavy user :) this thread has taught me something.

going to close this and add an example to CRAL

@swyxio swyxio closed this as completed Dec 6, 2018
@talves
Copy link
Contributor

talves commented Dec 6, 2018

Ok, so maybe we can come up with a solution based on a check that spits out a warning.

function createCallback(response) {
  return function callback(err, lambdaResponse) {
    if (err) {
      return handleErr(err, response);
    }

    // ***Checking for a callback without err AND without lambdaResponse***
    if (!lambdaResponse || !lambdaResponse.statusCode) {
      // Handle the error condition when no pending response
    }

    response.statusCode = lambdaResponse.statusCode;
    for (const key in lambdaResponse.headers) {
      response.setHeader(key, lambdaResponse.headers[key]);
    }
    response.write(
      lambdaResponse.isBase64Encoded
        ? Buffer.from(lambdaResponse.body, "base64")
        : lambdaResponse.body
    );
    response.end();
  }
}

@swyxio
Copy link
Contributor

swyxio commented Dec 6, 2018

ah i see @talves you want a nicer error message? sure I could do that. whats a nice error message? something like this?

    // ***Checking for a callback without err AND without lambdaResponse***
    if (!lambdaResponse) {
      throw new Error('No response detected from your handler lambda. You may have forgotten to call `callback` or return a Promise.')
    }
	if (!lambdaResponse.statusCode) {
		throw new Error('No statusCode detected in your handler lambda. You may have forgotten to return an object with a defined statusCode (usually 200 for success) in your `callback`.')
	}

@talves
Copy link
Contributor

talves commented Dec 6, 2018

Yeah, those may work. Do we need to check for a pending response before we throw the error. Let me test this out because it would end up throwing the same Warning as above. Maybe we could just put a response.end() before we throw the errors.

@swyxio
Copy link
Contributor

swyxio commented Dec 6, 2018

not sure what you mean by "throwing the same error" but i'll leave this open if you wanna send in a PR :)

@swyxio swyxio reopened this Dec 6, 2018
@talves
Copy link
Contributor

talves commented Dec 6, 2018

It might take me awhile to get to, but yeah, I will take a look.

Also, I did a promise example for the CRA netlify/create-react-app-lambda#17

@talves
Copy link
Contributor

talves commented Dec 6, 2018

@sw-yx I confirmed my statement about the warning when you use callback with an async handler and we throw the error.

Request from ::1: GET /joke
Type of Promise: function function
err: null
lambdaResponse undefined
(node:640) UnhandledPromiseRejectionWarning: Error: No response detected from your handler lambda. You may have forgotten to call `callback` or return a Promise.
    at callback (D:\Git\Netlify\lambda-functions\test-site\node_modules\netlify-lambda\lib\serve.js:27:9)
    at D:\Git\Netlify\lambda-functions\test-site\node_modules\netlify-lambda\lib\serve.js:54:21
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)
(node:640) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:640) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
response: { category: null,
  icon_url: 'https://assets.chucknorris.host/img/avatar/chuck-norris.png',
  id: 'v6Wo1N4KQQqvVeDBoSNiCg',
  url: 'https://api.chucknorris.io/jokes/v6Wo1N4KQQqvVeDBoSNiCg',
  value: 'Go to Joke #51, for this joke. This was hacked by Chuck Norris.' }
err: null
lambdaResponse { statusCode: 200,
  headers: { 'Content-Type': 'application/json' },
  body: '{"category":null,"icon_url":"https://assets.chucknorris.host/img/avatar/chuck-norris.png","id":"v6Wo1N4KQQqvVeDBoSNiCg","url":"https://api.chucknorris.io/jokes/v6Wo1N4KQQqvVeDBoSNiCg","value":"Go to Joke #51, for this joke. This was hacked by Chuck Norris."}' }
Response with status 200 in 946 ms.

Throwing the error without handling the response gives the warning but also returns the error message we throw. I will have to look at this when I get time.

All in all, this should be a confirmation that you CANNOT return a promise to the handler and use the callback together.

@swyxio
Copy link
Contributor

swyxio commented Dec 6, 2018

soo ok tell me what to do

@talves
Copy link
Contributor

talves commented Dec 6, 2018

We need to test 2 conditions at minimum:

  • Making the callback without any values (err, response), I think this will be handled
  • Calling an ansync handler and returning the callback.

I am not sure what to do, until I am sure that all entry points on the handler are tested because I do not want to introduce false messages to the developer.

@talves

This comment has been minimized.

@swyxio

This comment has been minimized.

@waynehoover

This comment has been minimized.

@talves

This comment has been minimized.

@waynehoover

This comment has been minimized.

@sidati
Copy link

sidati commented Feb 22, 2019

Any progress on this one ? 😄

@romseguy
Copy link

romseguy commented Mar 4, 2019

Same problem and resolved by removing callback here https://github.com/netlify/netlify-faunadb-example/blob/master/functions/todos-read-all.js#L21 and returning the plain object direclty.

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

10 participants