Skip to content

[release/7.0-rc2] [wasm] fix missing managed stack trace on managed exceptions marshaled to JS - #75799

Merged
carlossanlop merged 1 commit into
dotnet:release/7.0-rc2from
pavelsavara:backport/pr-75678-to-release/7.0-rc2
Sep 19, 2022
Merged

[release/7.0-rc2] [wasm] fix missing managed stack trace on managed exceptions marshaled to JS#75799
carlossanlop merged 1 commit into
dotnet:release/7.0-rc2from
pavelsavara:backport/pr-75678-to-release/7.0-rc2

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Sep 17, 2022

Copy link
Copy Markdown
Member

Backport of #75678 to release/7.0-rc2

/cc @pavelsavara@kg

Customer Impact

Fixes stack trace on managed stack trace, which was marshaled as JavaScript Error. The main impact is that unhanded exceptions from [JSExport] will be easier to diagnose than without the managed stack.

Testing

I done:

  • New unit tests
  • run unit tests on Chrome, FireFox and NodeJS
  • run full CI test suite for wasm on main branch
  • manual test

Risk

Low
There is probably no production code which would rely on this yet, as the [JSExport] interop is new in .Net 7.
The stack traces are targeted at human developer and relying on exact text of the stack trace is considered bad practice.

@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

Backport of #75678 to release/7.0-rc2

/cc @pavelsavara@kg

Customer Impact

Fixes stack trace on managed stack trace, which was marshaled as JavaScript Error. The main impact is that unhanded exceptions from [JSExport] will be easier to diagnose than without the managed stack.

Testing

I done:

  • New unit tests
  • run unit tests on Chrome, FireFox and NodeJS
  • run full CI test suite for wasm on main branch
  • manual test

Risk

Low
There is probably no production code which would rely on this yet, as the [JSExport] interop is new in .Net 7.
The stack traces are targeted at human developer and relying on exact text of the stack trace is considered bad practice.

Author:pavelsavara
Assignees:pavelsavara
Labels:

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

Milestone:7.0.0

@pavelsavara

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

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

@pavelsavara

Copy link
Copy Markdown
MemberAuthor

The issue is Failed to resolve SDK 'Microsoft.Build.NoTargets/3.5.0'

@carlossanlopcarlossanlop added the Servicing-consider Issue for next servicing release review label Sep 19, 2022
@carlossanlop

Copy link
Copy Markdown
Contributor

Needs Tactics approval. @pavelsavara please send the email to Tactics if you haven't yet.

@pavelsavara

Copy link
Copy Markdown
MemberAuthor

The CI failures are unrelated.

@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 19, 2022

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving based on approvals on the original PR for main.

@radical

Copy link
Copy Markdown
Member

The wasm/runtimeTests failures are unrelated.

kg
kg approved these changes Sep 19, 2022
@carlossanlop

Copy link
Copy Markdown
Contributor

And the failure to resolve Microsoft.Build.NoTargets/3.5.0 is this existing issue, which is a duplicate of this more generic one.

Signed off, approved, ready to merge. :shipit:

@carlossanlop
carlossanlop merged commit 3056135 into dotnet:release/7.0-rc2Sep 19, 2022
@pavelsavara
pavelsavara deleted the backport/pr-75678-to-release/7.0-rc2 branch October 13, 2022 10:04
@ghostghost locked as resolved and limited conversation to collaborators Nov 12, 2022
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

@pavelsavara@carlossanlop@radical@kg