feat: improve error handling in edge functions#246
Merged
eduardoboucas merged 6 commits intomainfrom Jun 2, 2025
Merged
Conversation
eduardoboucas
commented
Jun 2, 2025
|
|
||
| error.stack = `${prefix} ${error.stack}` | ||
|
|
||
| reject(error) |
Member
Author
There was a problem hiding this comment.
Start to reject the Promise if there has been an error rather than silently returning a successful response without any configs.
eduardoboucas
commented
Jun 2, 2025
| // The timeout imposed by the edge nodes. It's important to keep this in place | ||
| // as a fallback in case we're unable to patch `fetch` to add our own here. | ||
| // https://github.com/netlify/stargate/blob/b5bc0eeb79bbbad3a8a6f41c7c73f1bcbcb8a9c8/proxy/deno/edge.go#L77 | ||
| const UPSTREAM_REQUEST_TIMEOUT = 37_000 |
Member
Author
There was a problem hiding this comment.
Moved these timeouts upstream so we can override them as needed.
eduardoboucas
commented
Jun 2, 2025
| @@ -0,0 +1,23 @@ | |||
| export const getErrorResponse = (error: unknown) => { | |||
Member
Author
There was a problem hiding this comment.
This is the same format produced by the bootstrap layer, which is understood by the error page renderer.
serhalp
approved these changes
Jun 2, 2025
| import { fileURLToPath, pathToFileURL } from 'node:url' | ||
|
|
||
| import { renderFunctionErrorPage, type Geolocation } from '@netlify/dev-utils' | ||
| import { Logger, renderFunctionErrorPage, type Geolocation } from '@netlify/dev-utils' |
Member
There was a problem hiding this comment.
We should just enable verbatimModuleSyntax so this is automatic. I'll stop pointing these out for now. 😄
Suggested change
| import { Logger, renderFunctionErrorPage, type Geolocation } from '@netlify/dev-utils' | |
| import { type Logger, renderFunctionErrorPage, type Geolocation } from '@netlify/dev-utils' |
Comment on lines
+168
to
+180
| const initMessage = setTimeout(() => { | ||
| if (this.initialized) { | ||
| return | ||
| } | ||
|
|
||
| this.logger.log( | ||
| 'Setting up the Netlify Edge Functions environment. This may take up to a couple of minutes, depending on your internet connection.', | ||
| ) | ||
| }, 1_500) | ||
|
|
||
| const { denoPort, success } = await this.initialization | ||
| if (!success) { | ||
| clearTimeout(initMessage) |
Member
There was a problem hiding this comment.
nit:
Suggested change
| const initMessage = setTimeout(() => { | |
| if (this.initialized) { | |
| return | |
| } | |
| this.logger.log( | |
| 'Setting up the Netlify Edge Functions environment. This may take up to a couple of minutes, depending on your internet connection.', | |
| ) | |
| }, 1_500) | |
| const { denoPort, success } = await this.initialization | |
| if (!success) { | |
| clearTimeout(initMessage) | |
| const initMessageTimeout = setTimeout(() => { | |
| if (this.initialized) { | |
| return | |
| } | |
| this.logger.log( | |
| 'Setting up the Netlify Edge Functions environment. This may take up to a couple of minutes, depending on your internet connection.', | |
| ) | |
| }, 1_500) | |
| const { denoPort, success } = await this.initialization | |
| if (!success) { | |
| clearTimeout(initMessageTimeout) |
Comment on lines
+17
to
+21
| errors[name] = { | ||
| message: error.message, | ||
| name: error.name, | ||
| stack: error.stack, | ||
| } |
Member
There was a problem hiding this comment.
any chance any of these Errors have a cause property that we want to not discard?
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two separate commits: