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

Display more informative error information in TimeoutError response body #1460

Merged
merged 2 commits into from
Nov 10, 2020

Conversation

caesarsol
Copy link
Contributor

@caesarsol caesarsol commented Oct 21, 2020

- Summary

Closes #1459, problem described with detail in relative issue.
There are more elegant ways to solve this, but I would delay them to later times. This is kind of a quick but solid workaround.

I also removed the colored from the response body (ANSI chars), and changed the console log message to include the same exact failure message of the response: "Function invocation failed" (was slightly different, creating confusion).

- Test plan

Tested as described in #1459 with a TimeoutError, and by manually causing a new Error in the code.
Knowing what else can cause an error would help in triggering other possible error types.

- Description for the changelog

  • Error 500 "Function invocation failed" message is more informative in both response body and console log (for example a TimeoutError)

- A picture of a cute animal (not mandatory but encouraged)

cute cat

Thanks! ;)

@caesarsol caesarsol requested a review from a team as a code owner October 21, 2020 17:54
@caesarsol caesarsol changed the title Change handleErr() to display more informative error information Display more informative error information in TimeoutError response body Oct 21, 2020
@erezrokah erezrokah added the type: bug code to address defects in shipped code label Oct 21, 2020
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this @caesarsol, I believe you'll need to run npm run format to fix the linting errors on CI

@caesarsol
Copy link
Contributor Author

Sorry about that! I'll do whenever I find the time.

@caesarsol
Copy link
Contributor Author

Rewrote commit!

@caesarsol
Copy link
Contributor Author

Solved conflict and linting problem

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caesarsol sorry for the late follow up on this.
I made some additional fixes:

  1. When err is not a lambda error, it is a string, not an error instance - changed the code accordingly.
  2. When the error comes from lambda it was already logged by winston so no need to console.log it again (it is logged as JSON as the moment but that's another issue).
  3. Extracted the custom validation Netlify CLI does on lambda response to a separate function.

Please let me know what you think.

@erezrokah erezrokah merged commit 11828a2 into netlify:master Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Functions] Response body after Timeout is mangled and non-informative
2 participants