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

Bug: Home page display error when first loading #3951

Closed
zyhfish opened this issue Mar 5, 2024 · 9 comments
Closed

Bug: Home page display error when first loading #3951

zyhfish opened this issue Mar 5, 2024 · 9 comments

Comments

@zyhfish
Copy link
Contributor

zyhfish commented Mar 5, 2024

Reproduce Steps

  1. build the code in visual studio and run it.

Expected Result
Home page loaded without any error.

Actually Result
Home page display error when first loading, and it will display correctly after reload the page.
image

Exception from error logs
System.InvalidOperationException: There is already an open DataReader associated with this Connection which must be closed first.
at Microsoft.Data.SqlClient.SqlCommand.<>c.b__195_0(Task1 result) at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke()
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
--- End of stack trace from previous location ---
at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable1.AsyncEnumerator.InitializeReaderAsync(AsyncEnumerator enumerator, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func4 operation, Func4 verifySucceeded, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable1.AsyncEnumerator.MoveNextAsync()
at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable1 source, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable1 source, CancellationToken cancellationToken)
at Oqtane.Modules.HtmlText.Repository.HtmlTextRepository.<>c__DisplayClass8_0.<b__0>d.MoveNext() in E:\Git\zyhfish\oqtane.framework\Oqtane.Server\Modules\HtmlText\Repository\HtmlTextRepository.cs:line 61
--- End of stack trace from previous location ---
at Microsoft.Extensions.Caching.Memory.CacheExtensions.GetOrCreateAsync[TItem](IMemoryCache cache, Object key, Func`2 factory)
at Oqtane.Modules.HtmlText.Repository.HtmlTextRepository.GetHtmlTextsAsync(Int32 moduleId) in E:\Git\zyhfish\oqtane.framework\Oqtane.Server\Modules\HtmlText\Repository\HtmlTextRepository.cs:line 58
at Oqtane.Modules.HtmlText.Services.ServerHtmlTextService.GetHtmlTextAsync(Int32 moduleId) in E:\Git\zyhfish\oqtane.framework\Oqtane.Server\Modules\HtmlText\Services\HtmlTextService.cs:line 50
at Oqtane.Modules.HtmlText.Index.OnParametersSetAsync() in E:\Git\zyhfish\oqtane.framework\Oqtane.Client\Modules\HtmlText\Index.razor:line 28

@zyhfish
Copy link
Contributor Author

zyhfish commented Mar 5, 2024

Hi @sbwalker , should this be transient but not scoped so that the service will be new instance every time request to it:

services.AddScoped(serviceType, implementationType);

@sbwalker
Copy link
Member

sbwalker commented Mar 5, 2024

@zyhfish you are correct - fixed by #3953

@sbwalker sbwalker closed this as completed Mar 5, 2024
@sbwalker
Copy link
Member

sbwalker commented Mar 5, 2024

@zyhfish an error is still happening even after the change to use Transient scope. If you navigate between the Home / Private / MyPage pages after starting the framework, this method:

        public async Task<IEnumerable<Models.HtmlText>> GetHtmlTextsAsync(int moduleId)
        {
            return await _cache.GetOrCreateAsync($"HtmlText:{_siteState.Alias.SiteKey}:{moduleId}", async entry =>
            {
                entry.SlidingExpiration = TimeSpan.FromMinutes(30);
                return await _db.HtmlText.Where(item => item.ModuleId == moduleId).ToListAsync();
            });
        }

is throwing an error:

"Error Loading Content There is already an open DataReader associated with this Connection which must be closed first."

In the latest documentation I see these warnings for EF Core when it comes to Blazor:

"In server-side Blazor apps, DbContext isn't thread safe and isn't designed for concurrent use. The existing lifetimes are inappropriate for these reasons:

  • Singleton shares state across all users of the app and leads to inappropriate concurrent use.
  • Scoped (the default) poses a similar issue between components for the same user.
  • Transient results in a new instance per request; but as components can be long-lived, this results in a longer-lived context than may be intended.

By default, consider using one context per operation. The context is designed for fast, low overhead instantiation:

using var context = new MyContext();
return await context.MyEntities.ToListAsync();"

Note that the DBContext for the Html/Text module is Transient scoped currently. The problem is that in a multi-tenant environment you can't simply new up a DBContext - it requires more run-time awareness of the environment (ie. dependencies).

https://learn.microsoft.com/en-us/aspnet/core/blazor/blazor-ef-core?view=aspnetcore-8.0#new-dbcontext-instances

If EF Core cannot be used in these scenarios then we may need to revert back to using the HttpClient approach as it manages the scoping appropriately.

@sbwalker sbwalker reopened this Mar 5, 2024
@sbwalker
Copy link
Member

sbwalker commented Mar 5, 2024

This article provides guidance on EF Core DBContext, Blazor, and multi-tenancy:

https://learn.microsoft.com/en-us/ef/core/miscellaneous/multitenancy

Oqtane most likely needs a DBContextFactory which replicates the functionality of the existing DBContextBase for multi-tenancy but avoids DI and supports server-side Blazor. This approach would theoretically work for all render modes.

@sbwalker
Copy link
Member

sbwalker commented Mar 6, 2024

#3959 resolves the issue for the Html/Text module but is not a complete solution as OqtaneServiceCollectionExtensions.cs needs a way to call AddDbContextFactory dynamically

@sbwalker
Copy link
Member

sbwalker commented Mar 6, 2024

Recording some additional information for future context...

Let's say you have Page1 and Page2 in Oqtane and they each have their own Html/Text module assigned (ie. each module instance has a different ModuleId). The default component for the Html/Text module is the Index.razor component. So when Page1 is rendered, an Index component is created - which creates services and loads the content based on the ModuleId. When you navigate to Page2 you would assume that the previous component instance and its services are disposed and a new component instance and services are created. However this is NOT the case. Blazor notices that the component type is the same and is in the same location in the DOM so it re-uses the same component instance. So this means that a component can actually stay alive for MULTIPLE server requests. So this is why a DbContext which is Transient scoped can sometimes be re-used by a component and exhibit strange behavior and errors. This is why Microsoft recommends using a DbContextFactory for server-side Blazor.

@sbwalker
Copy link
Member

sbwalker commented Mar 7, 2024

If anyone wants to try to solve a .NET problem.... for auto registration of module services, Oqtane needs the ability to call AddDbContextFactory by providing a type dynamically...

For example, rather than specifying a static reference to a type in code:

services.AddDbContextFactory<HtmlTextContext>(opt => { }, ServiceLifetime.Transient);

We need to be able to pass a dynamic type:

var type = Type.GetType("HtmlTextContext");
services.AddDbContextFactory<type>(opt => { }, ServiceLifetime.Transient);

The challenge is there is no overload which allows this usage, so we would need to use reflection for both a generic method and a lambda parameter. Or we would need to create our own extension method which calls all of the same service registration methods which AddDbContextFactory calls internally.

@sbwalker
Copy link
Member

sbwalker commented Mar 8, 2024

I just realized that the IServerStartup interface can be used for registering the DbContextFactory. A module simply needs to provide a class implementing IServerStartup:

    public class HtmlTextDbContextFactory : IServerStartup
    {
        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            // not implemented
        }

        public void ConfigureMvc(IMvcBuilder mvcBuilder)
        {
            // not implemented
        }

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddDbContextFactory<HtmlTextContext>(opt => { }, ServiceLifetime.Transient);
        }
    }

@sbwalker sbwalker closed this as completed Mar 8, 2024
@thabaum
Copy link
Contributor

thabaum commented Mar 8, 2024

@sbwalker the force is with you!

Sorry, I had a more complicated solution in mind, nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants