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

catch JS exceptions when using InvokeAsync and throw them in .NET #41916

Merged
merged 1 commit into from
Jun 16, 2022

Conversation

campersau
Copy link
Contributor

@campersau campersau commented May 29, 2022

catch JS exceptions when using InvokeAsync and throw them in .NET

Exceptions in createJSCallResult and stringifyArgs were only thrown in JS before and .NET never received them.

Examples of exceptions which are now reaching .NET:

Fixes #40272 (references the last exception from the list above)
Fixes #41934 (exception in stringifyArgs)

@campersau campersau requested a review from a team as a code owner May 29, 2022 21:47
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label May 29, 2022
@@ -17,6 +17,20 @@ public JSObjectReferenceJsonConverterTest()
JsonSerializerOptions = DefaultWebAssemblyJSRuntime.Instance.JsonSerializerOptions;
}

[Fact]
public void Read_ReadsJson_IJSObjectReference()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated bonus test.

@Pilchie Pilchie added the area-blazor Includes: Blazor, Razor Components label May 31, 2022
@mkArtakMSFT
Copy link
Member

Thanks for your PR, @campersau.
@MackinnonBuck can you please review this? Thanks!

@campersau campersau force-pushed the IJSObjectReferenceNull branch 3 times, most recently from c720be5 to b2886c7 Compare June 8, 2022 19:16
@campersau campersau changed the title catch JS exceptions during createJSCallResult and throw them in .NET catch JS exceptions when using InvokeAsync and throw them in .NET Jun 8, 2022
@campersau
Copy link
Contributor Author

Updated to also catch exceptions during stringifyArgs which is the case in #41934 and added tests for synchronus Invoke to show that the behavior is the same as InvokeAsync, both throwing exceptions.

@campersau campersau force-pushed the IJSObjectReferenceNull branch 2 times, most recently from d8728f4 to 19d99c5 Compare June 9, 2022 06:40
Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

LGTM

@MackinnonBuck MackinnonBuck merged commit 7efa830 into dotnet:main Jun 16, 2022
@ghost ghost added this to the 7.0-preview6 milestone Jun 16, 2022
@MackinnonBuck
Copy link
Member

Thanks @campersau!

@ghost
Copy link

ghost commented Jun 16, 2022

Hi @MackinnonBuck. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@campersau campersau deleted the IJSObjectReferenceNull branch June 16, 2022 21:48
@datvm
Copy link

datvm commented Mar 27, 2023

Hi is this PR actually online yet? I am using .NET 7 Blazor WebAssembly (7.0.202) and I still have this issue. The code runs fine if JS returns something but does not returns when JS returns undefined (Error printed in Console but nothing happens on .NET side).

    public static async Task<T> GetGlobalObject<T>(this IJSRuntime js, string key) =>
        await js.InvokeAsync<T>(
            GetFullFuncName("getGlobalObj"),
            key);

    public static async Task<IJsWindowReference?> GetOpenerAsync(this IJSRuntime js)
    {
        var opener = await js.GetGlobalObject<IJSObjectReference?>("opener");
        return opener?.AsWindow(js);
    }
    getGlobalObj(key) {
        return globalThis[key];
    }

@ghost
Copy link

ghost commented Mar 27, 2023

Hi @datvm. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member
Projects
None yet
5 participants