Making ExceptionHelper friendly to Extensions

[rdohms]

I need to extend this class so I can add extra context information that is pertinent to our setup.

However since the client is protected, this is not possible.
For now i have made it protected so i can be accessed.

The second option if you do not like this, is to have a ExceptionHandler Interface available to we can then implement our own stuff and replicate functionality, or use composition.

[dcramer]

@rdohms is your goal to override the client? If so could you just do it via the configuration rather than overriding it on the class? Or is it that you simply want access to the client instance?

[rdohms]

No, I want to extend the exception listener to grab more information fr the request, problem is since the client property is protected I cannot call $this->client->extra_data() to add the data I need. I would need to implement a completely new listener and not reuse any of the original.

A protected getclient method would also suffice.

[dcramer]

Alright that's reasonable.

I will say though you likely would just want to augment the existing error. For example, you could specify 'send_callback' and add it to the exact event object thats relevant. Using the context methods will bind them for the remainder of the request, which sometimes is what you want, and other times may be incorrect.

[rdohms]

Thanks.
I agree but the current case fits this model. We want to grab data that is in the body of the request, not in POST data, so setting it when the request comes in and having it ready if we catch an exception makes sense.

[rdohms]

@dcramer thanks for the merge, any idea when you will tag a release?

[Addvilz]

I don't think this is the best way to achieve this. This will now expose what is basically a private "api" out for the extension. This means that after merging this we must maintain that the client variable is always there and always with the same name, same with the other variable you exposed - hence, if @dcramer does not revert this, we need some test for that then.

Much better way would be to have a request or other listener that has sentry client injected. From there then you can invoke sentry client and access the extra_data method.

Bad, but still better than this would be to expose a getter method, which later, if needed, could mutate to something else.

[rdohms]

If you want to make this better, then allow the exception_listener config to be a service. Because with the current construct it's I'm possible to replace of my constructor needs anything different.

Might also want to make some example implementations to help people looking for that extension point.

[Addvilz]

Sorry, I don't quite get you - what do you mean by having exception listener config as a service?

You can freely override this service, add or remove tags, or wrap this service in another one easily - Symfony container allows all that (app config has preference over bundle config), if that's what you meant. You can remove event listener tags with a container extension for this service if you like. Although, as previously mentioned, having another service that is invoked when request information is available and registers that information with Sentry client is much more optimal approach.