Skip to content

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

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

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Sep 17, 2022

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.

@pavelsavara pavelsavara added this to the 7.0.0 milestone Sep 17, 2022
@pavelsavara pavelsavara requested a review from kg September 17, 2022 17:51
@pavelsavara pavelsavara self-assigned this Sep 17, 2022
@pavelsavara pavelsavara requested a review from lewing as a code owner September 17, 2022 17:51
@ghost
Copy link

ghost commented Sep 17, 2022

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
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

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

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

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

@pavelsavara
Copy link
Member Author

The CI failures are unrelated.

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

@radical radical left a comment

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
Member

radical commented Sep 19, 2022

The wasm/runtimeTests failures are unrelated.

@carlossanlop
Copy link
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-rc2 Sep 19, 2022
@pavelsavara pavelsavara deleted the backport/pr-75678-to-release/7.0-rc2 branch October 13, 2022 10:04
@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2022
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