#3897 Fix error catching in blazor ApplicationContextManagers#3903
Conversation
async and match new error text after Microsoft changed it.
StefanOssendorf
left a comment
StefanOssendorf
left a comment
There was a problem hiding this comment.
Thanks for the changes :)
|
Thanks for the changes. Do you happen to know if it's possible to add tests for this behavior? |
|
Good question and that would be nice just in case some "helpful" person at MS decides to change the message again :-) Things are a bit fuzzy going back to 2022 but I "think" it was my HostedService in my Blazor project that was causing this to throw such that I noticed it. Obviously the BlazorExample throwing on the List page shows there are more ways to trigger it though. Anyway, my HostedService starts at project startup and runs on a timer outside of both HttpContext and a SignalR Circuit fetching a read-only list of site translation strings. After upgrading to CSLA 8.0.1 I get that error at startup (can't even get to my login/start page). So it may be possible to mimic that somehow in a test? My gut says yes it should be do-able. It would take some fooling around on a weekend to see. Should we put in an issue for it so we don't forget? |
|
@StefanOssendorf - git seems to think there is still an unresolved change request from you and I can't figure it out. Did I miss something? |
|
|
||
| //check for .net 8+ error text | ||
| bool newGetCalledOutsideRazorScopeError = message.Contains(nameof(AuthenticationStateProvider.GetAuthenticationStateAsync) | ||
| + " outside of the DI scope for a Razor component", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Having hard-coded english text seems problematic - I must think that this text will vary depending on the culture of the user and/or server?
There was a problem hiding this comment.
I wondered about that. Any ideas? The exception doesn't have anything else I can grab onto that I can see.
There was a problem hiding this comment.
There was a problem hiding this comment.
Stepping back for a moment...I guess the risk of simply testing for message.Contains(nameof(AuthenticationStateProvider.GetAuthenticationStateAsync) is low because this method is only hit on CTOR so it's not like it would be firing repeatedly over and over. What do you think about me removing everything but that on the check for the InvalidOperationException?
There was a problem hiding this comment.
wow, it is amazing that they accepted hard-coded text into the repo like that.
Why do we need to be aware of the specific text? Is there some way we can shield against future changes? This seems fragile to me.
|
@rockfordlhotka does that seem a little better? That would have gotten us through the recent language change. Combine that with the fact that the type of exception is checked for as well. |
rockfordlhotka
dismissed
StefanOssendorf’s stale review
Changes completed as requested
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
3 participants
make async and match new error text after Microsoft changed it old exception text. See discussion.
Not sure about the async part if I've got that right. But I tested it and things work fine.
After upgrading to CSLA 8.1.0, before this change, I would hit the exception immediately on blazor project start (probably because of my HostedService). After this fix I am able to run the blazor project without issue. All my thousands of tests pass too.
Fixes #3897
Fixes #3890
EDIT: I also used this to get your BlazorExample working to on the list page (according to @russblair repro steps here)