-
Notifications
You must be signed in to change notification settings - Fork 510
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
Bring back last_event_id
#3057
Bring back last_event_id
#3057
Conversation
0dbb7be
to
751f6ca
Compare
@@ -750,6 +763,9 @@ def capture_event( | |||
if self.transport is None: | |||
return None | |||
|
|||
if is_error: | |||
self._last_event_id = event_id |
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.
Do we want last_event_id
only to return the ID of the most recent error event? Or, should we expand it to all events? Error event seems like the better option to align with #3049
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, I think only errors is fine. Because the error ID is important for user feedback form.
@@ -709,6 +721,7 @@ def capture_event( | |||
|
|||
is_transaction = event_opt.get("type") == "transaction" | |||
is_checkin = event_opt.get("type") == "check_in" | |||
is_error = not is_transaction and not is_checkin |
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 guess this would also include profiles? Maybe make a check similar to the one for transaction?
I am going to rewrite this PR, since we should be setting the |
Closing in favor of #3064, which will add |
last_event_id
will return the ID of the most recently captured error eventCloses #3049