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

RaygunMessage Initializer Support #361

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Bio2hazard
Copy link

@Bio2hazard Bio2hazard commented Oct 12, 2017

Hey!

We use raygun a lot and love it, but ran into a lot of restrictions when trying to enrich our exception reports with information residing on the current thread.

Other libraries we use commonly have the concept of Initializers - customer-added code that runs directly after the message is created.

I have recreated that functionality by building out support for it in the base Raygun4Net project, and then implemented it in the Raygun4Net.WebApi project.

In order to ensure the initializers run on correct thread, I had to move the Threadpool to send the message to Raygun down the stack, after the message has been built. This also allows for removing the ThreadLocal RaygunRequestMessage, as we can now build it directly in the BuildMessage call.

I matched your code standards to the best of my abilities and documented all public calls. I also added a unit test that shows how to use it, and verifies the behavior.

Please let me know if you have any questions, or need me to change something!

This would also solve #347 & #297

Moved the ThreadPool further down the stack to run after the messages have been created.

Added initializers to the buildmessage call.
@QuantumNightmare
Copy link
Contributor

Hi Bio2hazard,

Thanks for the PR, we'll take a look when we have time and hopefully include it in the next release.

I've only had a quick look though this, it seems similar to the SendingMessage event that lets you perform additional logic when a message is about to be serialized and sent to Raygun. If you have not seen this, try it out and see if it can achieve the same behavior or not: https://raygun.com/docs/languages/net/webapi#modify-cancel-message

-Jason Fauchelle

@Bio2hazard
Copy link
Author

Hello Jason,

I am well aware of the SendingMessage event, but unfortunately it is not sufficient to meet my requirements ( and that of others, judging by the issue tracker ).

The key differences are:

  • For SendInBackground requests, the SendingMessage event runs on the background thread, and as such does not have access to call context. This is a problem as it makes it impossible to enrich the exception with request-related information.
  • Being an event handler, SendingMessage is limited in it's capabilities. Database access, dependency injection and calls to non-static application code is very difficult to safely accomplish.
  • Adding the proposed Initializer is a non-breaking change allowing for an additional extensibility point using user-provided code. This gives infinite freedom to enrich the exception with information. It can be kept simple as a initializer that simply runs on the same thread and has access to the HttpContext, or complicated by registering a per-request scoped factory delegate on the applications' dependency injection container and using that for initialization. It gives the user the freedom to do what they need to get information that will aid them in debugging.

Right now I can't even re-use a single RaygunClient instance and set the affected user from SendingMessage because I don't have access to the context that contains the user information. I am forced to create a new raygun client every time and set the user, but that does not let me set Custom data or tags.

Thank you for your time. I am hopeful to see this in the next release as it would make embedding debugging information in the exception indefinitely easier.

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

Successfully merging this pull request may close these issues.

2 participants