[GCPFunction] Failure to flush leads to cloud function hanging #4809

Activity

1. jtadmor

   added

   Type: Bug

   on Mar 28, 2022
2. getsantry

   added

   Status: Untriaged

   on Mar 28, 2022
3. jtadmor commented on Mar 28, 2022

   

   jtadmor

   on Mar 28, 2022

   Author

   More actions

   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.
4. lforst

   pinned this issue on Mar 28, 2022
5. lforst

   unpinned this issue on Mar 28, 2022
6. lforst commented on Mar 28, 2022

   

   lforst

   on Mar 28, 2022

   Contributor

   More actions

   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.
7. lforst

   added

   Package: serverless

   Status: Backlog

   and removed

   Status: Untriaged

   on Mar 28, 2022
8. jtadmor commented on Mar 28, 2022

   

   jtadmor

   on Mar 28, 2022

   Author

   More actions

   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.
9. lforst

   added

   Jira

   on Mar 29, 2022
10. lforst

    added

    Status: Confirmed

    on Apr 11, 2022
11. lforst commented on May 13, 2022

    

    lforst

    on May 13, 2022

    Contributor

    More actions

    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.
12. fpoppinga commented on May 30, 2022

    

    fpoppinga

    on May 30, 2022

    More actions

    We are observing the following problem, which seems to be related to this issue:

    When our sentry subscription runs out of transactions (i.e. we reach for example 100k transactions included in the minimum price business subscription), our functions start to hang all the time. There is a perfect correlation in our metrics between cloud functions hanging and sentry transactions running out.

    I suspect that something internally throws, when the transaction can not be reported and subsequently this issue is triggered.

    As a workaround you can buy additional sentry transactions (not ideal, I'd say), and we are looking into downgrading the sentry @sentry/serverless package.

    Maybe you can hotfix this in a release prior to v7?
13. lforst commented on May 30, 2022

    

    lforst

    on May 30, 2022

    Contributor

    More actions

    @fpoppinga that problem should also be resolved with the fix. The v7 release will most likely happen later today. I will check back with the team about cherry picking onto v6. For now the recommendation is to upgrade.

    🎉React with 🎉2Reacted by Finn Poppinga and Amir Hadi
14. fpoppinga commented on May 30, 2022

    

    fpoppinga

    on May 30, 2022

    More actions

    @lforst Thanks for getting back to me, alright, I'll look into upgrading to 7 ASAP then :)
15. lforst commented on Jun 1, 2022

    

    lforst

    on Jun 1, 2022

    Contributor

    More actions

    Closing this because we released v7 - feel free to ping me if there are any outstanding issues!
16. lforst

    closed this as completedon Jun 1, 2022