[GCPFunction] Failure to flush leads to cloud function hanging

[jtadmor]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/serverless

SDK Version

6.19.2

Framework Version

6.19.2

Link to Sentry event

No response

Steps to Reproduce

We have a project in which we happen to be over the transaction limit.

1. Call GCPFunction.wrapHttpFunction around any function.
2. Set tracesSampleRate: 1.0

See some errors like this show up in the logs:

"severity":"ERROR","message":"Sentry Logger [Error]: Error while sending event: SentryError: HTTP Error (429)
  at new SentryError (node_modules/@sentry/utils/dist/error.js:9:28)
  at ClientRequest.<anonymous> (node_modules/@sentry/node/dist/transports/base/index.js:213:44)
  at Object.onceWrapper (events.js:421:26)
  at ClientRequest.emit (events.js:314:20)
  at ClientRequest.EventEmitter.emit (domain.js:506:15)
  at HTTPParser.parserOnIncomingClient (_http_client.js:601:27)
  at HTTPParser.parserOnHeadersComplete (_http_common.js:122:17)
  at TLSSocket.socketOnData (_http_client.js:474:22)
  at TLSSocket.emit (events.js:314:20)
  at TLSSocket.EventEmitter.emit (domain.js:506:15)"}

Expected Result

Transactions / traces do not send, but functions continue to operate as expected.

Actual Result

Function times out.

[jtadmor]

Note: I am able to fix this issue by running the same wrapHttpFunction code locally, with this alteration to this portion

      void flush(options.flushTimeout)
        .catch((e) => {
           isDebugBuild() && logger.error(e);
           _end.call(this, chunk, encoding, cb);
        }).
        .then( // keep the rest of the code )

Basically, it seems like the issue is that it is possible not to call _end.call if flush errors in any way.

[lforst]

Thanks for writing in! I investigated a bit and I believe this error occurs when sentry returns a NOK response while we are flushing the event buffer with drain().

The following section in the promise buffer will simply return a rejected promise while no consumer of drain() actually expects it to reject.

sentry-javascript/packages/utils/src/promisebuffer.ts

Lines 92 to 98 in 4ab3abb

	void resolvedSyncPromise(item).then(() => {
	// eslint-disable-next-line no-plusplus
	if (!--counter) {
	clearTimeout(capturedSetTimeout);
	resolve(true);
	}
	}, reject);

I'm not totally sure whether that's actually the issue because I didn't try to repro, but if it is we should def fix it.

[jtadmor]

Thanks for looking into this so quickly! This mismatch in expectation makes sense. I think seeing the Sentry error in our logs is vaguely useful (or could be to others), so I'm not sure if the answer is just to swallow things locally or to explicitly .catch flush / drain calls everywhere. We have not noticed any other fallout from this issue, but will let you know if we do.

[lforst]

We just merged #5090 which will hopefully get rid of this problem entirely.

The SDK should not throw anymore when transmitting errors to Sentry fails. In case of an error, the SDK will simply log the error (if you set debug: true in the Sentry.init() options).

We are currently amidst preparing for the v7 release of the SDK - meaning this change will become available with the next beta or release candidate version of v7.