EdCharbeneau / BlazorSize

Blazor browser size interop for matchMedia and browser window size at runtime.
335 stars 38 forks source link

System.InvalidOperationException #98

Open newdeal30 opened 1 year ago

newdeal30 commented 1 year ago

Hi Ed!, First, thanks for this great library. I really enjoy your work. I issued a problem when working with the library on .net 7. The problem only accours when using Safari on Mac or IOS when the screen size is mobile. On other browsers or other screen sizes i did not have this problem. It happens on WebAssembly and on Blazor Server. Do you have a hint for me?

[08:22:35 DBG] Adding entry for MudBlazor.BrowserViewportSubscription/MudBlazor.MudDrawer. 1 total observers after add.
warn: Microsoft.AspNetCore.Components.Server.Circuits.RemoteRenderer[100]
      Unhandled exception rendering component: Collection was modified; enumeration operation may not execute.
      System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
         at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
         at BlazorPro.BlazorSize.MediaQueryService.Initialize(MediaQuery mediaQuery)
         at BlazorPro.BlazorSize.MediaQueryList.Initialize(MediaQuery mediaQuery)
         at BlazorPro.BlazorSize.MediaQuery.OnAfterRenderAsync(Boolean firstRender)
         at Microsoft.AspNetCore.Components.RenderTree.Renderer.GetErrorHandledTask(Task taskToHandle, ComponentState owningComponentState)
newdeal30 commented 1 year ago

I did a quick and dirty fix which is working for me. (try/catch at BlazorPro.BlazorSize.MediaQueryService.Initialize(MediaQuery mediaQuery)

Regards Michael

EdCharbeneau commented 1 year ago

That's strange behavior. This means that something is trying to modify the MediaQuery cache on MediaQueryList while it is initializing. There is only one moment where the initialize routine iterates the collection, and it does not have any add/remove routines inside. This means components must be getting constructed or disposed during initialization for some reason. I would probably need to see some Razor to understand this. Unfortunately, due to Apple's walled garden, I don't own a device to test this on.

I'm surprised BlazorSize continues to work after adding Try/Catch and ignoring the exception. It could lead to the improper state being reported or events not triggering. But if it works on your machine, I can't really argue with the solution 😁

Is there any chance you could recreate a basic implementation that doesn't have any proprietary code or dependencies and test it on Safari mobile? You can find a bunch of examples in https://github.com/EdCharbeneau/BlazorSize/tree/master/TestComponents

I'm curious if this is a Safari issue, BlazorSize issue, or something related to how your components are using BlazorSize (ex: with MudBlazor).

EdCharbeneau commented 1 year ago

Another solution, if you're willing to test it. Remove your try/catch and modify this line:

[ foreach (var item in cache.MediaQueries) { item.MediaQueryChanged(cache.Value);](https://github.com/EdCharbeneau/BlazorSize/blob/e4daa618d123c008acae6be4b5fafe7f2b127456/BlazorSize/MediaQuery/MediaQueryService.cs#L83-L85)

change cache.MediaQueries to cache.MediaQueries.ToList()

See if that resolves the issue, if it does I'll issue a patch ASAP.

newdeal30 commented 1 year ago

Hi Ed, I will test it on weekend and give you replay. Thanks

newdeal30 commented 1 year ago

I also have the exception on BlazorWebAssembly on MsEdge (but i hadn seen it, since there was no blazor error message in the ui - only webconsole) I tried to change the cache.MediaQueries.ToList(), but with no success. It is working when i decorate the MediaQueryService.Initialize AND MediaQueryList.MediaQueryChanged with try/catch. Maybe it has something to do with my constellation with MudBlazor (which is also using some MediaQueries)

image

I think we can close the issue, because of a special constellation of my enviroment. Thanks and regards Michael

newdeal30 commented 1 year ago

Would it be a problem to add the try/catch on the two methods on the official package? Then I could stay on the official source ;-)

EdCharbeneau commented 1 year ago

My worry is catching an error and swallowing it could lead to instability issues that would be hard to troubleshoot because the exception is hidden.

I'll have to find a way to make it optional, maybe some sort of compatibility flag that is an option parameter.

StevenRasmussen commented 1 year ago

I am running into this as well. I can reproduce on EDGE mobile... but also if I use EDGE on a desktop and use the developer tools and force it to emulate a mobile screen.

StevenRasmussen commented 1 year ago

I feel like the issue might be when you use this in conjunction with the AuthorizeView component.

Here is the code to my component that causes issues:

@using System.Security.Claims
@using BlazorPro.BlazorSize

@inject NavigationManager _navigationManager

<AuthorizeView>
    <Authorized>
        <div class="d-flex flex-row align-items-center gap-3">
            <div class="d-flex flex-row align-items-center gap-2 pointer-cursor" id="accountId" @onclick="@(() => _accountMenuOpen = !_accountMenuOpen)">
                <FluentIcon Icon="Icons.Filled.Size24.PersonCircle" />
                @if (!_isSmall)
                {
                    @context.User.FindFirstValue(ClaimTypes.GivenName)
                }
            </div>
            <div @onclick="@(() => _navigationManager.Logout())" style="cursor:pointer">
                Log out
            </div>
        </div>
    </Authorized>
    <NotAuthorized>
        <div class="d-flex flex-row align-items-center gap-3">
            <div class="pointer-cursor" @onclick="@(() => _navigationManager.NavigateToLoginPage())">login</div>
        </div>
    </NotAuthorized>
</AuthorizeView>

<MediaQuery Media="@Breakpoints.XSmallDown" @bind-Matches=_isSmall />

@code {
    private bool _accountMenuOpen = false;
    private bool _isSmall;

    private void NavigateToRoute(string route)
    {
        this._navigationManager.NavigateTo(route);
    }
}

If I comment out the <MediaQuery> component then I do not have the issue.

EdCharbeneau commented 11 months ago

@StevenRasmussen I was still unable to reproduce the error. I tried the sample from above, along with using the Edge mobile view. 🤷‍♂️

Sorry I can't help, this has been difficult to repro.

Mike-E-angelo commented 6 months ago

I am running into this issue. In my case I have three components on the same page that each have their own MediaQuery and for some reason they are stomping on each other and generating this error. I recently took out some asynchronous code that delayed the initialization for components while calls were made to the database. Now that this is gone this issue has presented itself.