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

System.InvalidOperationException #98

Open
newdeal30 opened this issue Sep 17, 2023 · 11 comments
Open

System.InvalidOperationException #98

newdeal30 opened this issue Sep 17, 2023 · 11 comments
Labels
bug Something isn't working more info needed Further information is requested about the problem statement

Comments

@newdeal30
Copy link

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
Copy link
Author

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

Regards Michael

@EdCharbeneau
Copy link
Owner

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
Copy link
Owner

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-L86C22)

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

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

@EdCharbeneau EdCharbeneau added bug Something isn't working more info needed Further information is requested about the problem statement labels Sep 18, 2023
@newdeal30
Copy link
Author

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

@newdeal30
Copy link
Author

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
Copy link
Author

newdeal30 commented Sep 21, 2023

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
Copy link
Owner

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
Copy link

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
Copy link

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
Copy link
Owner

@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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working more info needed Further information is requested about the problem statement
Projects
None yet
Development

No branches or pull requests

4 participants