@sentry/cloudflare crashes DurableObjects due to unsafe & unnecessary proxying of `waitUntil()`

[aroman]

Which SDK are you using?

@sentry/cloudflare

SDK Version

9.40.0

we are seeing sentry's cloudflare SDK frequently cause infinite recursion issues with DurableObjects.

production stacktrace:

source code referenced:

link to repo source: https://github.com/getsentry/sentry-javascript/blame/12528ec04ce752db6e800f2d577c3d7fcda3d24b/packages/cloudflare/src/flush.ts#L22

this code is broken and unnecessary

there is no reason (ever) to call waitUntil() inside a DurableObject. it is a no-op; please see the documentation

[linear]

JS-899 @sentry/cloudflare crashes DurableObjects due to unsafe & unnecessary proxying of `waitUntil()`

[aroman]

cc: @0xbad0c0d3, @andreiborza, @AbhiPrasad

[0xbad0c0d3]

@aroman can you, please, show steps to reproduce? I would agree it is not necessary to use waitUntil inside DO. But it is not prohibited / deprecated to use it.

Hibernation can only occur if all of the conditions below are true:

No setTimeout/setInterval scheduled callbacks are set, since there would be no way to recreate the callback after hibernating.
No in-progress awaited fetch() exists, since it is considered to be waiting for I/O.
No WebSocket standard API is used.
No request/event is still being processed, because hibernating would mean losing track of the async function which is eventually supposed to return a response to that request.

Without waitUntil we would need to call flush inside "setTimeout" callback or await it (we can not await inside sync "apply()")

So, may be you can share the problematic source code, or just share more info about the steps to be able to reproduce?
Anyway I'm going to investigate it shortly.

[aroman]

@0xbad0c0d3 thank you for your attention to this.

it's not easy to provide a repro because the error is not with my code 😅 i am not familiar with your codebase but my guess is that you're modifying the global context.waitUntil() without checking to see if it's already been instrumented/hijacked, which it well could be if multiple DurableObjects are running on the same isolate. this is nondeterministic based on cloudflare's internal compute scheduling/routing. just a guess. we are shipping a patch[1] to just completely disable the flushLock since it is unnecessary for DurableObjects which do not utilize the hibernation API, such as ours.

Again, I don't know much about your codebase or why you say you "can't await", but hijacking global APIs like waitUntil() seems like a much hackier and riskier approach than simply awaiting a fetch, which is surely "the right way to do it"

diff --git a/build/cjs/flush.js b/build/cjs/flush.js
index b00cfe27df64ea4bd730f61c2c44277fd5f3a541..cee82de571812f3424351c83a707e1289c637b8f 100644
--- a/build/cjs/flush.js
+++ b/build/cjs/flush.js
@@ -9,6 +9,10 @@ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
  * @return {FlushLock} Returns a flusher function if a valid context is provided, otherwise undefined.
  */
 function makeFlushLock(context) {
+  // Disabled due to https://github.com/getsentry/sentry-javascript/issues/17514
+  // The flushLock implementation causes infinite recursion in Durable Objects
+  return undefined;
+  
   let resolveAllDone = () => undefined;
   const allDone = new Promise(res => {
     resolveAllDone = res;
diff --git a/build/esm/flush.js b/build/esm/flush.js
index 348abe236208e425c26f1540ef2bf9662366edba..783b9e6867be44330fc97216c7a47a2cf81e6155 100644
--- a/build/esm/flush.js
+++ b/build/esm/flush.js
@@ -7,6 +7,10 @@
  * @return {FlushLock} Returns a flusher function if a valid context is provided, otherwise undefined.
  */
 function makeFlushLock(context) {
+  // Disabled due to https://github.com/getsentry/sentry-javascript/issues/17514
+  // The flushLock implementation causes infinite recursion in Durable Objects
+  return undefined;
+  
   let resolveAllDone = () => undefined;
   const allDone = new Promise(res => {
     resolveAllDone = res;