User is not populated properly in some cases

[mathieukempe]

User is not populated properly in some cases

[genne]

The main fix for it was to make sure you don't store the instance of HttpContext.Current (since it requires things to happen in a certain order and also won't work good for multiple requests), but instead always evaluate it. Also, not sure why HttpContext was set to null in all test cases, but had to remove it since it's a getter only by now.

Static properties/variables are always a pain 😄

Btw, PR here: #134

[asbjornu]

@mathieukempe Is this in any way improved in version 2.1 of SharpRaven?

[genne]

Any updates on this? I almost never get the correct user info in Sentry.

[asbjornu]

@genne Which version are you using?

[genne]

2.1.0 (nuget package)

[asbjornu]

@genne How are you capturing? Which method on ISentryClient are you using?

[genne]

I'm calling Capture(SentryEvent) on an instance of RavenClient.

[asbjornu]

@genne, @mathieukempe: Would any of you care to submit a pull request with the alleged fix? I don't quite understand how it can fix this problem, but if it does, I'll happily merge it!

[genne]

As mentioned by @mathieukempe, the order of the two lines must be swapped, since SentryUserFactory.Create() depends on SentryRequestFactory. Though, even if you fix that you still store the current HttpContext as a static instance which will not work when you have multiple requests occurring at the same time. You either need to use ThreadStaticAttribute before each static variable, or remove them completely and handle it in a better way.

I'll try to fix a PR just to get it working.

Further, you have a test case to make sure User is initialized correctly (Constructor_WithHttpContext_UsertIsNotNull), but

1. in SimulateHttpRequest you initialize User and Request differently from how it's actually initialized, and
2. you don't test multiple requests occurring at the same time.

[asbjornu]

@genne I'll be the first to admit that this part of the code is bad in every imaginable interpretation of the word. It's horrible. But I blame most of that on HttpContext.Current being static and requiring a reference to System.Web.dll. If you have ideas on how to improve this, you'd receive all the virtual ❤️ I can give and I'd be very happy. 😃 Looking forward to that PR!