feat: Remove deps and use Envelope Endpoint #252
Conversation
HazAT
changed the title
| @@ -24,6 +31,26 @@ export class NetTransport extends Transports.BaseTransport { | |||
| * @inheritDoc | |||
| */ | |||
| public async sendEvent(event: Event): Promise<Response> { | |||
| const envelopeHeaders = JSON.stringify({ | |||
| event_id: event.event_id, | |||
| sent_at: new Date(timestampWithMs() * 1000).toISOString(), | |||
There was a problem hiding this comment.
Would new Date().toISOString() suffice here?
There was a problem hiding this comment.
timestampWithMs has some internal logic how to get the "initial" clock reading. We use this clock everywhere so I would keep it like that for now.
src/main/transports/net.ts
Outdated
| }); | ||
| const itemHeaders = JSON.stringify({ | ||
| content_type: 'application/json', | ||
| type: event.type ?? 'event', |
There was a problem hiding this comment.
event.type can be something like "error" or "default", but those are not valid item types. Probably it's better to put this in a utility function that resembles this logic:
src/main/uploader.ts
Outdated
| type: 'event', | ||
| }); | ||
|
|
||
| const stat = await statAsync(minidumpPath); |
There was a problem hiding this comment.
You can avoid this stat, since you can simply query for minidumpContent.length below. That will return the binary size of the buffer. That way, you avoid differences between the FS metadata and the file contents, if any.
src/main/uploader.ts
Outdated
|
|
||
| // Too many requests, so we queue the event and send it later | ||
| if (response.status === CODE_RETRY) { | ||
| if (response.status === Status.RateLimit) { |
There was a problem hiding this comment.
Here, we have to make two changes:
- On a
429response, we should never retry. - On top of that, the envelope endpoint may return a
429, even if the event was accepted. Please check the correspondingX-Sentry-Rate-Limitsheader if a rate limit for the"error"category is supported, and ignore the ones for attachments for now.
We can later follow up with a more nuanced rate limiting logic.
src/main/uploader.ts
Outdated
| private async _toMinidumpRequest(event: Event, minidumpPath: string): Promise<SentryElectronRequest> { | ||
| const envelopeHeaders = JSON.stringify({ | ||
| event_id: event.event_id, | ||
| sent_at: new Date(timestampWithMs() * 1000).toISOString(), |
There was a problem hiding this comment.
It's probably worth noting that every time you retry a request, you have to generate a new sent_at timestamp. Otherwise, clock drift correction will see an old timestamp and assume your clock is lagging behind.
It seems that you do this correctly, can you confirm? I'd suggest to add a comment at some appropriate place for the future.
… into feat/envelopes
This greatly reduced bundle size and make the minidump upload a bit more robust.
Size of bundle before:
Size after:
This will become a major version bump because it will only work with on-premise Sentry
20.6.0or older since we use the envelope endpoint. https://develop.sentry.dev/sdk/envelopes/