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

Toolchain deps update #408

Merged
merged 3 commits into from
Mar 22, 2024
Merged

Toolchain deps update #408

merged 3 commits into from
Mar 22, 2024

Conversation

guybedford
Copy link
Collaborator

Updates to the latest toolchain dependencies including Wasmtime 19.

Also includes some features and fixes:

  • Refined error handling resolving Non-conformant exceptions should unwrap as traps #405. JS errors from imported JS functions now only coerce into results when the top-level result accepts a string error type, otherwise the JS error will trap.
  • Fixes a regression with the timeout API
  • Fixes the connection timeout support to pass the new Wasmtime test for that

@guybedford guybedford merged commit bcb28d9 into main Mar 22, 2024
8 checks passed
@guybedford guybedford deleted the deps-update branch March 22, 2024 18:20
@@ -127,7 +124,7 @@ export async function createHttpRequest(
host: authority.split(":")[0],
port: authority.split(":")[1],
path: pathWithQuery,
timeout: connectTimeout && Number(connectTimeout),
timeout: connectTimeout && Number(connectTimeout / 1_000_000n),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the firstByteTimeout and betweenBytesTimeout need this division step as well?

if (firstByteTimeout) res.setTimeout(Number(firstByteTimeout));
if (betweenBytesTimeout)
res.once("readable", () => {
res.setTimeout(Number(betweenBytesTimeout));
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great catch, thanks, added #409.

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

Successfully merging this pull request may close these issues.

2 participants