josephwoodward / GlobalExceptionHandlerDotNet

Exception handling as a convention in the ASP.NET Core request pipeline
MIT License
269 stars 32 forks source link

ASP.Net Core 2.1 Issue #18

Closed jculverwell closed 6 years ago

jculverwell commented 6 years ago

Just upgraded to ASP.Net Core 2.1 and got this error:

InvalidOperationException: An error occurred when configuring the exception handler middleware. Either the 'ExceptionHandlingPath' or the 'ExceptionHandler' option must be set in 'UseExceptionHandler()'.

My setup before was this:

 app.UseExceptionHandler().WithConventions(x => {
                x.ContentType = "application/json";
                x.MessageFormatter(s => JsonConvert.SerializeObject(new
                {
                    Message = "Internal Server Error"
                }));
                x.OnError((exception, httpContext) =>
                {
                    Log.Error(exception, "WebAPI Global Exception Handler");
                    return Task.CompletedTask;
                });
            });

Adding app.UseExceptionHandler("/error") as below seems to fix it.

app.UseExceptionHandler("/error").WithConventions(x => {
                x.ContentType = "application/json";
                x.MessageFormatter(s => JsonConvert.SerializeObject(new
                {
                    Message = "Internal Server Error"
                }));
                x.OnError((exception, httpContext) =>
                {
                    Log.Error(exception, "WebAPI Global Exception Handler");
                    return Task.CompletedTask;
                });
            });

Just wanted to give you a heads up and check the workaround makes sense. Thanks for a great repo.

josephwoodward commented 6 years ago

Great, thanks for reporting this @jculverwell, I'll take a look.

alhardy commented 6 years ago

Was curious about this also

josephwoodward commented 6 years ago

There's a bit more about why this was changed here.

I've come to the conclusion that this library's config should live separate to the UseExceptionHandler() endpoint. I've got a new release coming out in the next few days that doesn't change any behaviours, but it goes from:

app.UseExceptionHandler().WithConventions(...);

to

app.UseGlobalExceptionHandler(...);

It still uses the UseExceptionHandler under the hood, but now it's required to have a string passed to it, it feels very jarring and unclear.

slang25 commented 6 years ago

It's worth noting that this was changed in 2.1 because leaving it empty is dangerous. By default in 2.0 and below it will use the current path as the error handler path, so for us we had POST requests being "replayed" and things getting saved to the database twice etc...

josephwoodward commented 6 years ago

so for us we had POST requests being "replayed" and things getting saved to the database twice etc...

Was GlobalExceptionHandler causing them to be posted twice?

slang25 commented 6 years ago

No, just using using UseExceptionHandler with no parameter did it

josephwoodward commented 6 years ago

I've now released a beta package which resolves this (it's a major version bump as it includes some breaking changes, but they're just method renames). If you'd like to give it a try I'd love to hear how you get on with it.

The new version can be found here https://www.nuget.org/packages/GlobalExceptionHandler/4.0.0-beta1 and the release notes here.