User context in laravel 5.4 about sentry-laravel HOT 14 CLOSED

Hi!

For adding context we have an example for a middleware (but you can place the code that sets the user context anywhere) that adds user context or any other context you like.

https://github.com/getsentry/sentry-laravel#adding-context

But in Laravel >5.2 nothing should have changed in using Auth::user() so I'm a bit puzzled what it is that is not working, can you elaborate?

from sentry-laravel.

thanks @stayallive for you reply. i will be more accurate:

1. i install sentry via composer, i set providers and alias in config app
2. i set new report function in Handler.php as it is in install instructions, set env
3. so far everything is running as it should

next i wanted to add user context by middleware as you have in docs, i did but \Auth::user() is in laravel > 5.2 not created yet here. so let's have example.

when you have some syntax error or so in controller, it goes to sentry middleware and try to add user context, but user does not exists yet and \Auth::user() is null and auth()->check() is null - everytime.

you can see changes in auth (from L 5.3) handling here: https://laravel.com/docs/5.3/upgrade#5.3-session-in-constructors

from sentry-laravel.

A syntax error is not something that is easily catched, but I assume on syntax error you don't need the user 😉 How Laravel works I don't think it's possible to add the user context using a middleware since the controller is included before the middleware are run so the syntax error happens before any middleware had a chance to run. I don't think there is a place in Laravel where the session started and initialized before a syntax error is thrown, try it with an Exception.

But for other errors, is your Sentry middleware after the \Illuminate\Session\Middleware\StartSession middleware? You should add the Sentry middleware in your http Kernel (after the session middleware) not in your controller.

I am using the exact code shown in the docs in multiple Laravel 5.4 apps (the example came from me) so it should work for you to.

from sentry-laravel.

I think the middleware should be included as part of Sentry then you just add instructions to add it to the middleware list - It's quite confusing that there is a config option for user_context but only seems to work in Laravel 4.2.

from sentry-laravel.

Also, it's recommended to add it under a middleware but can't the user just get set inside your exception handler just before you captureException ? Is there any strong reason why you shouldn't do that? Unless some other part of your app uses sentry to capture then it would make sense... but all exceptions should be caught by the handler

from sentry-laravel.

The user_context option works for 4.2 and above (tested all the way to 5.4) as long as you want to get the user context from the the default guard. Is it not working for you in 5.x?

Yes you can also add the user context right before you send the errors of the Sentry but that adds an added panelty of setting the user context multiple times (if you get multiple errors in a sessions). And some users sent their app()->log('error', 'Ooops...') to Sentry which does not go through the exception handler, but they do want user context or environment tags/context. That is why we recommend doing it as early as possible so all the ways you can use Sentry work, adapt it for your needs 😉

from sentry-laravel.

That's good point about app()->log makes sense to set it in a middleware for that reason. Really wish Laravel had a Session::afterLoad(function().... type thing as an alternative but meh.

And no, the user_context thing does not work in Laravel 5.x for me at least. Inside the service provider there is bindEvents() which gets called on boot which seems to resolve sentry immediately - this may explain why Auth is not available as at that point the session hasn't been loaded. Does it work for you?

from sentry-laravel.

Personally I think SentryLaravelEventHandler should be refactored slightly so that it doesn't need the raven client - it should just be a repository of information for the logged events, then when the exception is captured to sentry it "attaches" all the data to the raven client.

At the moment, the information is attached straight away to the raven client as soon as the events are fired - this is kind of a waste because an exception might not even occur or be reported, yet it's still loading sentry and attaching stuff to it for every request!

from sentry-laravel.

I see where the problem could be, thanks for pointing it out. I will try to recreate this later in a fresh Laravel install.

The SentryLaravelEventHandler depends on the client so it can add breadcrumbs as they come to make sure breadcrumbs are also sent on app()->log() calls if the user uses that 😄
This way a breadcrumb never get's lost because somehow the breadcrumb collector could not attach it before events get sent.

from sentry-laravel.

Log::info() can still be supported too, you could listen for illuminate.log and then attach all the data to sentry if bound. Ideally exceptions should be a rare occurrence and initialising sentry client every single time even when no logs are produced seems a bit unnecessary to me. Also do you really want manual log calls to report to Sentry? Probably not. Dunno, maybe I'll fork the library and go with a different implementation.

from sentry-laravel.

We are accepting PR's @garygreen 😉

EDIT: Do keep in mind that people might still want to use the Sentry client outside the Laravel methods and might also want breadcrumbs to show up there.

from sentry-laravel.

Ha ok well maybe I'll send a PR then, though I do think we have fundamental different views on how the flow should be - I'm more of a "defer everything until needed" kinda guy and your approach is "process everything right now just in case" which is the safest way to be, but I think reasonable assumptions can be made about how/when things get reported to Sentry, and any edge case can still be easily supported 😄

from sentry-laravel.

This is always a thin line to walk, especially for something that could be mission critical like Sentry, if you miss a piece of the puzzle it could set you back hours potentially.

But maybe you have some great ideas on how to not be like the "process it NOW" and more towards the "maybe later when we need it". Happy to discuss them in a PR or another issue since we are moving away from the original problem of this issue ;)

from sentry-laravel.

@garygreen & @matomoravcik can you guys check out #81, this should fix the problems on Laravel >=5.3 regarding the user context.

from sentry-laravel.