elsa-core icon indicating copy to clipboard operation
elsa-core copied to clipboard

HttpEndpointMiddleware - When it fails issue

Open matt4446 opened this issue 2 years ago • 4 comments

This is the first time this one has popped up in a year of using Elsa 2...

Note - Im stuck on 2.3 until I can test the latest preview builds, but just checking this one won't cause any other issues down the line. The following error began from a request to a SignalR hub (a few hundred will be trying to get back in contact) to reconnect to the site. Restarting fixed the error below.

HttpEndpointMiddleware ->

The following exception caused the site to appear offline as no further requests could complete. The switch from an In memory to a provider with persistence will have a slightly different outcome when I finally get to upgrade but some other things come to mind.

  • It would be effective if the middleware was configurable so that it can only act on certain routes eg /elsa-endpoint/**/*. It creates extra work that the majority of requests don't need. Switching to minimal APIs might also be an interesting solution as well (but a different package), but can minimal endpoints (minimal apis .NET 6) get added/modified during the publishing of a workflow ... I'm less knowledgeable on that?.
  • Failure shouldn't throw on the request - might be debatable.
  • What could be done to heal itself short of restarting
System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method531644(Closure , Bookmark )
   at Elsa.Persistence.Specifications.Specification`1.IsSatisfiedBy(T entity)
   at Elsa.Persistence.Specifications.OrSpecification`1.IsSatisfiedBy(T entity)
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Elsa.Persistence.InMemory.InMemoryStore`1.FindManyAsync(ISpecification`1 specification, IOrderBy`1 orderBy, IPaging paging, CancellationToken cancellationToken)
   at Elsa.Services.Bookmarks.BookmarkFinder.FindBookmarksAsync(String activityType, IEnumerable`1 bookmarks, String correlationId, String tenantId, CancellationToken cancellationToken)
   at Elsa.Services.Workflows.WorkflowLaunchpad.CollectResumableAndStartableWorkflowsAsync(WorkflowsQuery query, CancellationToken cancellationToken)
   at Elsa.Services.Workflows.WorkflowLaunchpad.FindWorkflowsAsync(WorkflowsQuery query, CancellationToken cancellationToken)
   at Open.Linq.AsyncExtensions.Extensions.ToList[TSource](Task`1 source)
   at Elsa.Activities.Http.Middleware.HttpEndpointMiddleware.InvokeAsync(HttpContext httpContext, IOptions`1 options, IWorkflowLaunchpad workflowLaunchpad, IWorkflowInstanceStore workflowInstanceStore, IWorkflowRegistry workflowRegistry, IWorkflowBlueprintReflector workflowBlueprintReflector, IHttpEndpointAuthorizationHandler authorizationHandler, IEnumerable`1 contentParsers)
   at Microsoft.AspNetCore.Authorization.Policy.AuthorizationMiddlewareResultHandler.HandleAsync(RequestDelegate next, HttpContext context, AuthorizationPolicy policy, PolicyAuthorizationResult authorizeResult)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task)
   at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.HandleException(HttpContext context, ExceptionDispatchInfo edi)
   at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware.<Invoke>g__Awaited|6_0(ExceptionHandlerMiddleware middleware, HttpContext context, Task task)
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT`1.ProcessRequestAsync()

matt4446 avatar Mar 18 '22 11:03 matt4446

A year of Elsa 2 - very nice! 😃

It would be effective if the middleware was configurable so that it can only act on certain routes eg /elsa-endpoint/**/*. It creates extra work that the majority of requests don't need.

Absolutely - when you upgrade, you can configure the path prefix like this:

services.AddElsa(elsa => elsa.AddHttpActivities(options => options.BasePath = "/elsa-endpoint"));

The Elsa.Samples.Server.Host sample project uses /workflows by default as the route prefix.

Switching to minimal APIs might also be an interesting solution as well (but a different package), but can minimal endpoints (minimal apis .NET 6) get added/modified during the publishing of a workflow ... I'm less knowledgeable on that?.

Maybe, interesting to look into.

Failure shouldn't throw on the request - might be debatable.

With later versions, the HTTP Endpoint Middleware component returns a 500 status code when an unhandled exception occurs. This response can be overtaken by the application using a custom error handler if you want.

What could be done to heal itself short of restarting

I'm surprised this is even necessary - I'd expect a crashing HTTP request to kill just that request and not cause the server to choke. If you can provide steps to reproduce this with latest Elsa code from master, I'd be very interested in trying it out and look for a remedy.

sfmskywalker avatar Mar 18 '22 19:03 sfmskywalker

Thanks, I must have missed the configuration on my search.

As for killing all requests - It hasn't happened before, but once is enough to make it interesting. Two hours and 30000 error messages later it will make a challenging soup to unravel, but all seem to fail on the same error with little hint at its cause. Adding in the base path should remove the majority of requests that failed I would think, which would have reduced the errors to a few dozen failed triggered endpoints. If I can reproduce it reliably I'll let you know. It will be on my todo list.

matt4446 avatar Mar 21 '22 11:03 matt4446

Has the prefix been fixed since 2.3. Without the BasePath it works fine?

Without BasePath: image

image

Bookmark goes in as follows: image

With the prefix, it chops off the basePath and then doesn't find the bookmark resulting in a 404 image

image

image

after which the hash doesn't match any current bookmark.

matt4446 avatar Mar 21 '22 13:03 matt4446

Looks like the bookmark provider should not store the prefix portion of the path, but strip it and store the remainder of the path.

sfmskywalker avatar Mar 23 '22 10:03 sfmskywalker