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

[release/7.0] [wasm] Catch error from loading "node:crypto" module #79351

Closed

Conversation

maraf
Copy link
Member

@maraf maraf commented Dec 7, 2022

Backport of #78916 to release/7.0

Customer Impact

Based on node documentation, there might be a build of node without "node:crypto". In such a case the module import will throw an exception. In #78696 (backport #78766) we load "node:crypto" module during the runtime startup and in this PR we are catching such exception and rethrowing it only when user tries to use the API.

It restores the behavior before #78696 when crypto API is not available.
It should be included in the same release as #78696 (backport #78766).

Testing

Manual

Risk

None

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

* Catch error from loading node:crypto module.
* Throw error with explanation when crypto module is not available.
* Fix providing error throwing polyfill.
@maraf maraf added Servicing-consider Issue for next servicing release review arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript labels Dec 7, 2022
@maraf maraf added this to the 7.0.x milestone Dec 7, 2022
@maraf maraf requested a review from lewing as a code owner December 7, 2022 16:48
@maraf maraf self-assigned this Dec 7, 2022
@ghost
Copy link

ghost commented Dec 7, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #78916 to release/7.0

Customer Impact

Based on node documentation, there might be a build of node without "node:crypto". In such case the module import will throw an exception. In #78696 we load "node:crypto" module during the runtime startup and this PR we are catching such exception and rethrow it only when user tries to use the API.

It restores the behavior before #78696 when crypto API is not available.
It should be included in the same release as #78696.

Testing

Manual

Risk

None

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: maraf
Assignees: maraf
Labels:

Servicing-consider, arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 7.0.x

@marek-safar marek-safar added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 8, 2022
@carlossanlop carlossanlop deleted the branch dotnet:backport/pr-78696-to-release/7.0 January 4, 2023 20:16
@carlossanlop
Copy link
Member

I accidentally pressed Close instead of Squash+Merge. I will resubmit this backport PR and merge it. Apologies.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2023
@maraf maraf deleted the WasmBackport78916 branch June 21, 2024 11:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants