Skip to content

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

Merged
carlossanlop merged 1 commit into
dotnet:release/7.0from
carlossanlop:backport/pr-78696-to-release/7.0
Jan 5, 2023
Merged

[release/7.0] [wasm] Catch error from loading "node:crypto" module#80249
carlossanlop merged 1 commit into
dotnet:release/7.0from
carlossanlop:backport/pr-78696-to-release/7.0

Conversation

@carlossanlop

Copy link
Copy Markdown
Contributor

Backport of #78916 to release/7.0

Important: This is a resubmission of the previous backport PR submitted by @maraf that I accidentally closed instead of squash+merged. I was unable to reopen the PR because I had deleted the branch. This PR has already been approved by Tactics.

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.
@ghost

ghost commented Jan 5, 2023

Copy link
Copy Markdown

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

Important: This is a resubmission of the previous backport PR submitted by @maraf that I accidentally closed instead of squash+merged. I was unable to reopen the PR because I had deleted the branch. This PR has already been approved by Tactics.

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.

Author:carlossanlop
Assignees:carlossanlop, maraf
Labels:

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

Milestone:7.0.3

@carlossanlop

Copy link
Copy Markdown
ContributorAuthor

Approved by Tactics (7.0.3).
Signed off by area owners.
No OOB changes needed.
The CI failure seems to be a sudden crash of the test process.
Ready to merge. :shipit:

@carlossanlop
carlossanlop merged commit 7a88642 into dotnet:release/7.0Jan 5, 2023
@carlossanlop
carlossanlop deleted the backport/pr-78696-to-release/7.0 branch January 5, 2023 22:51
@ghostghost locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-System.Runtime.InteropServices.JavaScriptServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@lewing@pavelsavara@maraf