Skip to content

[wasm] Catch error from loading "node:crypto" module - #78916

Merged
maraf merged 3 commits into
dotnet:mainfrom
maraf:WasmFixNodeCrypto
Nov 29, 2022
Merged

[wasm] Catch error from loading "node:crypto" module#78916
maraf merged 3 commits into
dotnet:mainfrom
maraf:WasmFixNodeCrypto

Conversation

@maraf

@marafmaraf commented Nov 28, 2022

Copy link
Copy Markdown
Member

According to "node:crypto" documentation, there can be build of node without this module. We silently ignore the error with loading the module until some API requiring it is called.

This restores the behavior before #78696

@marafmaraf added this to the 8.0.0 milestone Nov 28, 2022
@maraf
maraf requested a review from lewing as a code ownerNovember 28, 2022 15:21
@marafmaraf self-assigned this Nov 28, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details

According to "node:crypto" documentation, there can be build of node without this module. We silently ignore the error with loading the module until some API requiring it is called.

Author:maraf
Assignees:maraf
Labels:

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

Milestone:8.0.0

@maraf

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm-libtests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment threadsrc/mono/wasm/runtime/polyfills.ts Outdated
@maraf
maraf merged commit 83cfc12 into dotnet:mainNov 29, 2022
@maraf
maraf deleted the WasmFixNodeCrypto branch November 29, 2022 16:58
@maraf

maraf commented Dec 1, 2022

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3593244549

@github-actions

Copy link
Copy Markdown
Contributor

@maraf backporting to release/7.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Catch error from loading node:crypto module.
Using index info to reconstruct a base tree...
M	src/mono/wasm/runtime/polyfills.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/polyfills.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/polyfills.ts
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Catch error from loading node:crypto module.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions

Copy link
Copy Markdown
Contributor

@maraf an error occurred while backporting to release/7.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

maraf added a commit to maraf/runtime that referenced this pull request Dec 7, 2022
* Catch error from loading node:crypto module.
* Throw error with explanation when crypto module is not available.
* Fix providing error throwing polyfill.
@ghostghost locked as resolved and limited conversation to collaborators Dec 31, 2022
@carlossanlop

Copy link
Copy Markdown
Contributor

Backport was resubmitted here: #80249

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@maraf@carlossanlop@pavelsavara