[Bug]: JSException from Blazorise modal

[njannink]

[Bug]: JSException from Blazorise modal

[njannink]

I guess the element is null here somehow

Blazorise/Source/Blazorise.Material/wwwroot/modal.js

Line 7 in b62023c

const modalBody = element.querySelector('.modal-body');

[njannink]

Possible a race condition here when disposing/navigating away.

Blazorise/Source/Blazorise/Components/Modal/Modal.razor.cs

Line 312 in b62023c

await JSModalModule.OpenModal( ElementRef, ScrollToTop );

[stsrki]

We would probably need to add a null check on a bunch of places.

[David-Moreira]

Yep as we've seen most of the js exceptions are just race conditions where the element has disappeared when something was about to be invoked.

Again I'd probably not only add the null check but also the try/catch all we've talked about on some other thread. I know try/catching all might seem not like a good idea, but realistically do we ever want to stop an user's application with one of these js exceptions?

[njannink]

biggest issue with these unhandled JSExceptions is that the CircuitHost dies and you loose your Blazor server session forcing the user to reload the page

[stsrki]

@David-Moreira Please try it for the coming 1.6 release. I would not change it too much in 1.5.

[David-Moreira]

I'd enable this by default and introduce a Blazorise option to be able to turn it off. I don't think a user will ever want to turn it off, but just in case... Agree?
Something like SafeJsInvoke //Whether to safely invoke internal javascript. Will ignore any exceptions that might be thrown as part of the javascript invoke process.

[stsrki]

Hmm. Can you explain how that will look in the code? Pseudo code is good.

[David-Moreira]

What do you mean?
It's an option, it either calls the current version or it calls the try catch all version.

if (safejsinvoke)
     InvokeSafe();
else
     InvokeWhateverWeHaveNow();

I'm not very familiar with the internals of our js invokes, so only when I start working will I see if there's any technical difficulty, but I don't expect any, it should be like that, the most centralized we can make it.

[stsrki]

Yes, but we already have InvokeSafeVoidAsync and InvokeSafeAsync, which should be safe already. That is why I'm confused about why we need an extra layer of "safe".

[David-Moreira]

Does this seem safe to you? hehe

[David-Moreira]

Whatever "safe" approach we have currently, is not safe in the manner that you can actually call it and trust that you won't have an exception.

[stsrki]

Does this seem safe to you? hehe

Haha, absolutely not. Just have a quick prototype in a few places and create a draft PR so that we can see how it looks. Making it optional is also a good idea, as you said.

[David-Moreira]

Does this seem safe to you? hehe

Haha, absolutely not. Just have a quick prototype in a few places and create a draft PR so that we can see how it looks. Making it optional is also a good idea, as you said.

Haha, yes.
Imagine you add a SafeReadFile(); and whenever you tried reading a file and it was locked or you had no permissions it threw an exception instead of like returning null? I don't think that would be expected behaviour of something "safe"?

Anyway, yes, I don't expect that there will be many places to change? But again, I'm not very familiar yet about those internals. I'd expect to only have to change in one or two places? Anyway, I'll provide a POC as suggested.