feat(transport): Add client report hook to makeTransport
#5008
Conversation
size-limit report 📦
|
lforst
force-pushed
the
lforst-make-transport-client-report-hook
branch
from
ebf0996 to
ebc938a
Compare
packages/utils/src/envelope.ts
Outdated
| const map = { | ||
| session: 'session', | ||
| sessions: 'session', | ||
| attachment: 'attachment', | ||
| transaction: 'transaction', | ||
| event: 'error', | ||
| client_report: 'internal', | ||
| user_report: 'default', | ||
| } as const; |
There was a problem hiding this comment.
nit: Let's extract this out to a top level constant
There was a problem hiding this comment.
Would you replace the entire function with the top level constant or would you just move it in outer scope?
There was a problem hiding this comment.
Either works for me - your call here.
| @@ -22,11 +22,18 @@ export function addItemToEnvelope<E extends Envelope>(envelope: E, newItem: E[1] | |||
| } | |||
|
|
|||
| /** | |||
| * Get the type of the envelope. Grabs the type from the first envelope item. | |||
| * Convenience function to loop through the items and item types of an envelope. | |||
| * (This function was mostly created because working with envelope types is painful at the moment) | |||
There was a problem hiding this comment.
on purpose because they are tuple types :P - but very fair
Introducing something like forEachEnvelopeItem is actually good behaviour because we intentionally want to treat the envelope data structure as a void pointer (only allow functions to access and mutate it).
There was a problem hiding this comment.
Interesting. Did not know that - thanks for bringing it up!
packages/types/src/transport.ts
Outdated
| @@ -14,7 +16,9 @@ export type TransportMakeRequestResponse = { | |||
|
|
|||
| export interface InternalBaseTransportOptions { | |||
| bufferSize?: number; | |||
| recordDroppedEvent?: (reason: EventDropReason, dataCategory: DataCategory) => void; | |||
There was a problem hiding this comment.
Should we just make this required?
There was a problem hiding this comment.
As discussed: yeah we should as clients should always pass this in
This PR introduces an internal option to inject a hook/callback into the transports, that is responsible for collecting client reports within the transport itself.
We are now also dropping individual envelope items that are rate limited, just like we do in the python SDK.
For more context: #5002
Ref: getsentry.atlassian.net/browse/WEB-775