-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore: Bump volta to node 20 #15315
chore: Bump volta to node 20 #15315
Conversation
@@ -199,6 +199,10 @@ export function extractType(ex: Error & { message: { error?: Error } }): string | |||
export function extractMessage(ex: Error & { message: { error?: Error } }): string { | |||
const message = ex?.message; | |||
|
|||
if (isWebAssemblyException(ex)) { | |||
return 'wasm exception'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This failed on node versions > 18, also tested it in integration tests.
Decided to hardcode it, as opposed to ex.toString() which returns [object WebAssembly.Exception
size-limit report 📦
|
@@ -207,11 +211,6 @@ export function extractMessage(ex: Error & { message: { error?: Error } }): stri | |||
return message.error.message; | |||
} | |||
|
|||
// Emscripten sets array[type, message] to the "message" property on the WebAssembly.Exception object | |||
if (isWebAssemblyException(ex) && Array.isArray(ex.message) && ex.message.length == 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we leave this for legacy stuff? Not sure if/how relevant this is...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm yeah I updated it to fallback to 'wasm exception' now
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
This PR bumps our volta field to use Node 20, which will be required for building upcoming SDK versions (Nest@11, RR@7)