ref: Add tests for breadcrumbs #2620
Conversation
| /** | ||
| * Tells the backend to send this event | ||
| * @param event The Sentry event to send | ||
| */ | ||
| protected _sendEvent(event: Event): void { | ||
| this._getBackend().sendEvent(event); | ||
| } | ||
|
|
There was a problem hiding this comment.
New internal function, helper for that.
| promisedEvent.then(event => { | ||
| eventId = this.captureEvent(event, hint, scope); | ||
| }); |
There was a problem hiding this comment.
@kamilogorek No idea why we didn't do it like this in the first place.
Now everything goes through captureEvent as it's supposed to be.
There was a problem hiding this comment.
Same here. Whole method can be written as:
const promisedEvent = // ...;
return promisedEvent.then(event => this.captureEvent(event, hint, scope);
| logger.error(reason); | ||
| this._processing = false; | ||
| .then(event => { | ||
| eventId = this.captureEvent(event, hint, scope); |
There was a problem hiding this comment.
It looks like eventId is always overwritten? Even if hint.event_id is provided? If so, we can:
return this._getBackend()
.eventFromException(exception, hint)
.then(event => this.captureEvent(event, hint, scope));Also this_processing = true; is already called inside captureEvent so we can most likely remove it from here? Same goes for eventId extraction, as it's done in captureEvent too.
There was a problem hiding this comment.
This is tricky so I would leave it like this, because depending on if there is really an async call in between this is true or not.
_isProcessing should also be called before first thing since stuff in captureX can be async
This adds tests for adding Sentry internal breadcrumbs.
This is a follow up from #2615.