-
Notifications
You must be signed in to change notification settings - Fork 39
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
Only include sessions data when sessions are available #84
Conversation
+1 |
@erwineverts Thank you for the PR. The fix looks fine. Can you take a look at the failing test? |
Maybe all it needs is rebase master. |
OK this seems to be the same as #90. |
Any plans to merge this PR ? That obviously solves the problem with Lumen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good. It is make sense that checking the session if set first before getting all the session in case that the application is Laravel Lumen.
@waltjones Any chance that this is accepted and merged? |
this fix was originally proposed on issue rollbar#83 and PR rollbar#84 and rollbar#90 without further interaction/response from the original contributor.
Hey @erwineverts @neochief @darkcarnage @villaflor @tristanjahier Seems that the branch is no longer available but I just merged #106 which duplicates this PR. |
@bxsx Thank you! I've been using Sentry for the past couple of years. But great to see progress on this bug. |
Thanks @erwineverts Cheers |
No description provided.