-
Notifications
You must be signed in to change notification settings - Fork 185
Add support for subscription to certain life cycle events #47
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
Merged
Jean85
merged 16 commits into
getsentry:master
from
swquinn:add-support-for-event-subscribers
May 17, 2017
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
a377c2a
Add support for subscription to certain life cycle events
swquinn 9a5aec3
Fix bad white space that failed due to PHP CS checker
swquinn 50b1ca7
Use getMock instead of createMock for compatibility with PHPUnit
swquinn ebe0e3c
Remove unneccessary space for marker interface definition
swquinn 4a4ff4b
Include PRE_CAPTURE example in README.md
swquinn cd17601
Change order of arguments for exception listener
swquinn f7c8d8d
Change example exception listener and add note
swquinn e075f15
Add the SentryExceptionListenerInterface
swquinn bc1648f
Clean up comments, remove unnecessary comments
swquinn d921cc5
Remove null checks around EventDispatcher
swquinn 0ec27dd
Fix failing build due to missing public modifiers on interface
swquinn 5068851
Fix extraneous new line causing linter to fail
swquinn 812007b
Remove null check that was missed in previous commit
swquinn 1b8c381
Add requirement that exception listener implement interface
swquinn 5eb1a36
Remove currently unnecessary interface
swquinn 78ebf11
Merge branch 'master' into add-support-for-event-subscribers
swquinn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?php | ||
|
||
namespace Sentry\SentryBundle\Event; | ||
|
||
use Symfony\Component\EventDispatcher\Event; | ||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; | ||
|
||
class SentryUserContextEvent extends Event | ||
{ | ||
private $authenticationToken; | ||
|
||
public function __construct(TokenInterface $authenticationToken) | ||
{ | ||
$this->authenticationToken = $authenticationToken; | ||
} | ||
|
||
public function getAuthenticationToken() | ||
{ | ||
return $this->authenticationToken; | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
src/Sentry/SentryBundle/EventListener/SentryExceptionListenerInterface.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
<?php | ||
|
||
namespace Sentry\SentryBundle\EventListener; | ||
|
||
use Symfony\Component\Console\Event\ConsoleExceptionEvent; | ||
use Symfony\Component\HttpKernel\Event\GetResponseEvent; | ||
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; | ||
|
||
interface SentryExceptionListenerInterface | ||
{ | ||
|
||
/** | ||
* Used to capture information from the request before any possible error | ||
* event is encountered by listening on core.request. | ||
* | ||
* Most commonly used for assigning the username to the security context | ||
* used by Sentry for each request. | ||
* | ||
* @param GetResponseEvent $event | ||
*/ | ||
public function onKernelRequest(GetResponseEvent $event); | ||
|
||
/** | ||
* When an exception occurs as part of a web request, this method will be | ||
* triggered for capturing the error. | ||
* | ||
* @param GetResponseForExceptionEvent $event | ||
*/ | ||
public function onKernelException(GetResponseForExceptionEvent $event); | ||
|
||
/** | ||
* When an exception occurs on the command line, this method will be | ||
* triggered for capturing the error. | ||
* | ||
* @param ConsoleExceptionEvent $event | ||
*/ | ||
public function onConsoleException(ConsoleExceptionEvent $event); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
|
||
namespace Sentry\SentryBundle; | ||
|
||
/** | ||
* Event names that are triggered to allow for further modification of the | ||
* Raven client during error processing. | ||
*/ | ||
class SentrySymfonyEvents | ||
{ | ||
|
||
/** | ||
* The PRE_CAPTURE event is triggered just before the client captures the | ||
* exception. | ||
* | ||
* @Event("Symfony\Component\EventDispatcher\Event") | ||
* | ||
* @var string | ||
*/ | ||
const PRE_CAPTURE = 'sentry.pre_capture'; | ||
|
||
/** | ||
* The SET_USER_CONTEXT event is triggered on requests where the user is | ||
* authenticated and has authorization. | ||
* | ||
* @Event("Sentry\SentryBundle\Event\SentryUserContextEvent") | ||
* | ||
* @var string | ||
*/ | ||
const SET_USER_CONTEXT = 'sentry.set_user_context'; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
You are adding a mandatory reference to service but permit service to be null in class itself. This is contradictory because Symfony will error out if event_dispatcher is not present in this case. Either make this service optional or disallow null in the constructor.
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.
I already commented on that up above. I agree, but the service should be taken for granted since we require
symfony/symfony
, so I would prefer no nullable in the class.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.
Sorry missed it. Yes, I reckon non-nullable parameter in the constructor would be the correct way to go then.
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.
I made the recommended change to the code (removing the
null
checks since we're requiring the event dispatcher). I guess I wasn't really thinking, and I also didn't notice the@?
prefix for services. I don't know if I've ever used that before, but I do most of my service definitions in XML in my own projects, so it is possible I just never knew about that with the YAML. I'm assuming that the@?
allows for a non-mandatory reference and null values to be passed into the function/constructor/whatever?Any way, the changes can be found in this commit: d921cc5
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.
Yes, the
@?
is used to silence failures for missing references and passing a null instead.