feat: Add attachments API

[timfish]

Closes#4996

This PR:

• Adds Attachment interface and extends AttachmentItemHeaders with required fields
• Adds addAttachment, getAttachments and clearAttachments to Scope
• Adds attachments: Attachment[] to EventHint
• In the private capture/process/prepare/send methods in the BaseClient, the EventHint argument has moved as it's no longer optional
  • We can't mutate the hint if it's undefined!
• Copies attachments from the scope to the hint in _prepareEvent
• Adds optional textEncoder to InternalBaseTransportOptions and overrides this in the node client to support node 8-10
  • Manually pass new TextEncoder() in many of the tests so they pass on node.js 8-10
• Adds binary serialisation support for envelopes
  • serializeEnvelope returns string | Uint8Array which all transports supported without modification
  • Defaults to concatenating strings when no attachments are found. String concatenation is about 10x faster than the binary serialisation
• Rewrites parseEnvelope in testutils.ts so that it can parse binary envelopes
  • Is there a common test utils so this can be used elsewhere?

Bonus

• This will significantly simplify sending minidumps from the Electron SDK

[AbhiPrasad]

We could move the envelope serialisation to the client so we can have multiple implementations?

I like this idea. I think that it should be client defined, and we pass it into the transport as an option. We did something similar with @lforst's work on client reports. See: #5008

// core/src/baseclient.tsthis._transport=options.transport({serializeEnvelope: mySpecificSerializeEnvelopeImpl,
...options.transportOptions,
url,});

What to do about attachment rate limits?

The changes in #5008 should mean that we respect rate limits per envelope items. See:

sentry-javascript/packages/core/src/transports/base.ts

Lines 46 to 54 in 1c5bff0

	// Drop rate limited items from envelope
	forEachEnvelopeItem(envelope,(item,type)=>{
	constenvelopeItemDataCategory=envelopeItemTypeToDataCategory(type);
	if(isRateLimited(rateLimits,envelopeItemDataCategory)){
	options.recordDroppedEvent('ratelimit_backoff',envelopeItemDataCategory);
	}else{
	filteredEnvelopeItems.push(item);
	}
	});

This will significantly simplify sending minidumps from the Electron SDK

Awesome ❤️

This was a quick first pass - I'll come back and do another review tomorrow when I get some time :)

[timfish]

We don't just need TextEncoder when serialising the envelope, we also need it outside the transport when creating the attachment envelope items. Attachments must include a length (in case that attachment contains newlines) and the easiest way to know the byte length of a utf8 string is to just convert it to bytes.

[timfish]

@AbhiPrasad I think this is ready for a 2nd pass.

It definitely still needs some tests to cover the changes around mutable hints + beforeSend + event processors.

[AbhiPrasad]

Were we able to test if babel will polyfill TextEncoder if they set the according browserslist?

[AbhiPrasad]

Why are we changing the func signature here?

[timfish]

serializeEnvelope now returns string | Uint8Array. In this case it will never actually be a Uint8Array because there's no attachment but both sendBeacon and fetch take body: string | Uint8Array so it seemed the best thing to do.

[timfish]

Were we able to test if babel will polyfill TextEncoder if they set the according browserslist?

It looks like babel now just expects you to import core-js if you want things polyfilled.

[AbhiPrasad]

Awesome, I think after the transport changes get extracted, we should be good to go to merge this in!

[timfish]

I think after the transport changes get extracted, we should be good to go to merge this in!

That is done already!

[AbhiPrasad]

I think after the transport changes get extracted, we should be good to go to merge this in!

That is done already!

Woooooops

[AbhiPrasad]

I think this is a good starting point for us - @getsentry/team-web-sdk-frontend I would appreciate another set of eyes to confirm!

[lforst]

Looks great. Thank you Tim!

[timfish]

How quickly can we get this into a release so I can test it all in this PR 😆

[AbhiPrasad]

I think we can merge tommorow, so get it out in a release by end of this week!

[pnutmath]

After upgrading sentry to v7 getting below error. I am using sentry with node express project - @timfish@AbhiPrasad

 message: 'finalScope.getAttachments is not a function or its return value is not iterable',
code: undefined,
sentry: '888a4d2d434b4e1c9b5dcea80eb99791',
stack: 'TypeError: finalScope.getAttachments is not a function or its return value is not iterable\n' +
' at NodeClient._prepareEvent (/srv/server/node_modules/@sentry/core/cjs/baseclient.js:403:69)\n' +
' at NodeClient._prepareEvent (/srv/server/node_modules/@sentry/node/cjs/client.js:151:18)\n' +
' at NodeClient._processEvent (/srv/server/node_modules/@sentry/core/cjs/baseclient.js:598:17)\n' +
' at NodeClient._captureEvent (/srv/server/node_modules/@sentry/core/cjs/baseclient.js:554:17)\n' +
' at NodeClient.captureEvent (/srv/server/node_modules/@sentry/core/cjs/baseclient.js:147:12)\n' +
' at NodeClient.captureEvent (/srv/server/node_modules/@sentry/node/cjs/client.js:93:18)\n' +
' at Hub._invokeClient (/srv/server/node_modules/winston-sentry-log/node_modules/@sentry/hub/dist/hub.js:384:35)\n' +
' at Hub.captureEvent (/srv/server/node_modules/winston-sentry-log/node_modules/@sentry/hub/dist/hub.js:171:14)\n' +
' at Transaction.finish (/srv/server/node_modules/@sentry/tracing/cjs/transaction.js:142:22)\n' +
' at Immediate.<anonymous> (/srv/server/node_modules/@sentry/node/cjs/handlers.js:51:21)\n' +
' at processImmediate (internal/timers.js:464:21)\n' +
' at process.topLevelDomainCallback (domain.js:152:15)\n' +
' at process.callbackTrampoline (internal/async_hooks.js:128:24)'

[lforst]

Hi @pnutmath I moved your comment into a separate issue #5229