MindscapeHQ / raygun4net

Raygun provider for .NET
https://raygun.com
MIT License
126 stars 93 forks source link

RaygunClient.SendInBackground (AspNetCore) and IRaygunUserProvider missing HttpContext #536

Closed isak1080 closed 3 months ago

isak1080 commented 5 months ago

Describe the bug While upgrading to Raygun4net 11 from 9.x I had to move away from IRaygunClientProvider to something else.

I now setup my own IRaygunUserProvider which uses IHttpContextAccessor to grap the user info from the HTTP request.

My RaygunClient correctly gets my custom user provider in its constructor.

The problem is when I call RaygunClient.SendInBackground - the call to IRaygunUserProvider.GetUser is done on a worker thread, which means it cannot get the HttpContext.

How is this supposed to work? My current workaround is to simply build the message myself using the RaygunMessageBuilder but then what's the point of doing all this setup with user provider etc.?

phillip-haydon commented 5 months ago

Hello!

The message is built before being queued and sent so it should always have access to the HttpContext.

Are you able to show some code or explain how I ca. reproduce the issue you’re running into?

Thank you.

isak1080 commented 5 months ago

That's not what I'm seeing. GetUser is called on a worker thread thanks to Task.Run

image

See the attached visual studio project that reproduces the issue: RaygunClientHttpContextBug.zip

phillip-haydon commented 5 months ago

Thank you for the project reproducing the issue, its helpful!

So we will prob need to tweak things a bit to resolve this, similar to what we do for getting the request/response information up front before passing it on.

There's a couple of work arounds I can offer in the short term:

1) Use .SendAsync(...)

This builds the message and sends it right away so it doesn't lose context.

2) Use the middleware, when you don't want to capture explicitly you can use the middleware which will pass the http context to SendInBackground.

3) Use the SendInBackground the middleware uses which causes the message to be built up front.

app.MapGet("/crash", (RaygunClient client, IHttpContextAccessor accessor) =>
{
    try
    {
        return 100 / int.Parse("0");
    }
    catch (Exception ex)
    {
        client.SendInBackground(ex, new List<string>(), accessor.HttpContext);
        return -1;
    }
});

What happens in this particular SendInBackground is it builds the message then queues it for delivery, but looking at the code we pass the build message as a delegate and that causes it to lose context. If I remember correctly the reasoning for that was under heavy load (millions of exceptions being thrown) it can chew alot of memory enqueuing all those messages.

By deferring the message building it didn't duplicate alot of the data so it used a fraction of the memory. But in hindsight if millions of exceptions are happening the website is probably dying at that point and the memory is the least of ones concerns. We also have a limit on the queue so it should discard and not exhaust the memory available anyway.

We will look into fixing this up and getting it tested etc, hopefully the work-arounds suffice for now, sorry you ran into this.

phillip-haydon commented 3 months ago

@isak1080 I re-tested your sample app in 11.0.4-pre-1 and the HttpContext is correctly available now. Once we finish testing dependent providers for Maui and Serilog we will get this released.

phillip-haydon commented 3 months ago

I've released v11.1.0 which should resolve this issue, let us know if it persists.