fix: gracefully fail Edge Functions in case of deno binary issues #4685
Conversation
This file contains 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
jackiewmacharia
changed the title
📊 Benchmark resultsComparing with 9dffdb0 Package size: 226 MB⬇️ 0.00% decrease vs. 9dffdb0 Legend
|
2 tasks
danez
reviewed
Jun 9, 2022
Co-authored-by: Daniel Tschinder <231804+danez@users.noreply.github.com>
…b.com:netlify/cli into fix/gracefully-fail-in-case-deno-binary-issue
eduardoboucas
previously approved these changes
Jun 15, 2022
|
@eduardoboucas suggestions applied and as per usual review reset 😅
|
| [headers.Passthrough]: 'passthrough', | ||
| [headers.RequestID]: generateUUID(), | ||
| [headers.IP]: LOCAL_HOST, | ||
| try { |
There was a problem hiding this comment.
I'm sorry if this wasn't clear in my previous comment, but I was trying to suggest that you only wrap the Promise.all call in the try/catch, not the entire block. Otherwise we're back to the problem of wrapping too much code and potentially swallowing errors.
Something like:
let ops
try {
ops = await Promise.all([
getGeoLocation({ mode: geolocationMode, offline, state }),
server,
])
} catch (err) {
error(err, { exit: false })
hasServerError = true
return
}
const [geoLocation, { registry }] = opsThere was a problem hiding this comment.
Aaah I see, done.
|
@eduardoboucas this one is now updated with edge-bundler changes and is now ready. |
jackiewmacharia
added
automerge
jackiewmacharia
deleted the
fix/gracefully-fail-in-case-deno-binary-issue
branch
bluejacket-c4
pushed a commit
to code-423n4/code423n4.com
that referenced
this pull request
Mar 9, 2023
the backstory is that Netlify CLI has trouble installing deno on my 64-bit ARM Linux VM (aarch64), see details on netlify/cli#4685 via netlify/cli#4621 unfortunately this didn't really fix the problem for me (should have been part of the 10.6.2 netlify-cli release netlify/cli#4729) but seems to be what other devs are using locally anyway so it seemed reasonable to bump it here the new @types/node dev dep is to avoid a (new?) unmet peer dependency warning with the new cli package
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #4621
Requires netlify/edge-bundler#42
netlify devbreaks in case one runs it on a platform (e.g linux aarch64) not yet officially supported by deno, to avoid this we need to gracefully fail Edge Functions making it possible to continue usingnetlify dev.To test this:
fix/add-error-state-to-after-downloadbranch of edge-bundler; runnpm installandnpm run buildin edge-bundler to set it up; edit CLI's package.json to use the cloned version of edge-bundler eg"@netlify/edge-bundler": "file:../edge-bundler"if (downloadedVersion !== undefined) {to cause the binary download step to throw an error.await this.writeVersionFile(downloadedVersion || '')to get past TypeScript type check errorsnpm run buildto make changes available CLI~/netlify/cli/bin/run devError string for review:
› There was a problem setting up the Edge Functions environment and it's unfortunately not possible to run Edge Functions from the CLI on this platform. More on supported platforms here: https://deno.land/manual/getting_started/installation. Error: <error.message from error object>Screenshot:
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)
