Missing FetchEvent.respondWith() error/warning

I am here sending a request for a feature/bug fix that I hope somebody will respondWith( ; )

From my tests it seems that FetchEvent.respondWith() must be called synchronously. Which makes sense because “the runtime proxies the request to the origin”, and you don’t want to wait for that. I think I have seen an Error message in the console about this, but I cannot seem to reproduce it. But I can reproduce this Error silently, so I go with that.

Below is an entire worker that demonstrate the problem, you can also view it on cloudflareworkers.com.

addEventListener('fetch', async e => {
    console.log('one');
    await new Promise(r=>setTimeout(r, 0)); //during this await, Implicit Response occurs.

    //Implicit response means that cloudflare worker passes the task of producing a response
    //to the cloudflare cdn (subsystem).

    //1. The Cloudflare worker will at this point no longer be able to call e.respondWith(..).
    //Sometimes (but this is hard to reproduce), calling e.respondWith(..) after this point
    //will trigger an Error. But very often, the e.respondWith(..) just fails silently.

    //2. After the Implicit Response occurs, an Invisible Timer for a Cleanup Bomb will start ticking.
    //The Invisible Timer is set to ~30ms (~100ms when worker is created).

    //The Cleanup Bomb has not exploded yet, and so this statement works.
    console.log('two');
    //If we call FetchEvent.respondWith now, it fails. Silently.
    try {
      e.respondWith(new Response('nothing, and no error message'));  
      //Bug 1: Can we please get an error thrown here?
    } catch (err){
      console.log('missing', error)
    }

    //Just checking that the FetchEvent.respondWith(..) is ok.
    try {
      e.respondWith(new Response('error message')); //Not a problem: InvalidStateError is thrown.
    } catch(err) {
      console.log(err.name, err.message);
    }

    //Playing with the Cleanup Bomb. Is it the red wire? or is it the green wire?
    await new Promise(r=>setTimeout(r, 30));
    console.log("maybe you see it, maybe you don't");
    //Problem 2: 
    //There is no clear indication about when the worker is cleaned up, hence the name CleanupBomb.
    //This might cause problems because you see a behavior in your initial tests,
    //then you make your assumption about how things work, but then
    //next time it behaves differently, and now you think the bug is caused by something else.
    
    //Yes. If you use FetchEvent.waitUntil(..), then you control the CleanupBomb.
    //But my point is not how to control cleanup. That is fine.
    //My point is that the CleanupBomb is confusing. It should either:
    //1. blow up (remove all tasks from the event loop) once Implicit Response is activated and no waitUntil is active
    //2. warn that the Implicit Response has started and that the CleanupBomb will occur (a console.warn?)
    //3. document its behavior.
    //Imho, a verbose #2 seems like a quick fix, which includes an element of documentation, and a nice compromise.
});

I have added explanation of the problem and my request in the code as comments.

After playing around with the problem a little, I made this polyfix:

//polyfix for missing error when calling respondWith after ImplicitResponse 
const addEventListenerOG = addEventListener;

addEventListener = function(type, cb) {
  if(type !== 'fetch')
    return addEventListenerOG(type, cb);
  const wrapper = function(fetchEvent){
    let active = true;
    const respondWithOG = fetchEvent.respondWith;
    fetchEvent.respondWith = function(promise){
      active = false;
      respondWithOG.call(fetchEvent, promise);
    };
    Promise.resolve().then(function(){
      if(!active)
        return;
      console.warn('Implicit Response: all tasks in the event loop triggered by this fetch event will be deleted in approx 30ms (100ms worker initialized).');
      fetchEvent.respondWith = function(){
        throw new Error('FetchEvent.respondWith() must be called synchronously from the event listener function.');
      };
    });
    cb(fetchEvent);
  }
  return addEventListenerOG(type, wrapper);
}
//polyfix end

addEventListener('fetch', async e => {
    console.log('one');
    //comment in the line below to test the polyfix use case when a response is given in.
    //e.respondWith(new Response('hello sunshine'));  
    await new Promise(r=>setTimeout(r, 0)); //during this await, Implicit Response occurs.
    try {
      e.respondWith(new Response('nothing, and no error message')); //throws an error
    } catch (err){
      console.log(err.name, err.message);
    }
    await new Promise(r=>setTimeout(r, 0));
    console.log('two');
    await new Promise(r=>setTimeout(r, 30));
    console.log('maybe');
    await new Promise(r=>setTimeout(r, 65));
    console.log('maybe when worker is instantiated');
    await new Promise(r=>setTimeout(r, 150));
    console.log('should not happen');
});

I am not sure to understand the problem, but when you create a Promise, you explicitely have to encapsulate the code in a resolve/reject function, then inside the encapsulated code, call the resolve(…) callback function and give it the result you want to return.

If you do not call the resolve(…) callback function, your Promise never fullfill (it never returns, it never ends).

const result = await new Promise((resolve, reject) => {
  setTimeout(() => {   resolve('foo');   }, 0);
});

Look at the resolve(‘foo’)

If you want to signal success, call resolve(…)
If you want to signal failure, call reject(…)

The built-in feature respondWith doesn’t have callback, that’s the point of it, being able to respond to the client before the request has completed in the worker, thus, it doesn’t work with await as the OP mentioned. As far as i can gather, the polyfix he’s created will wrap it into a promise so that it can be used as a promise inside the function.

The Promise+setTimeout is fine. When you do a await new Promise(r=> setTimeout(r, 1000)), you pass the r function as the argument to the setTimeout which then calls it when its time. You can test it out in the links above.

1 Like

The problem is the silence of the cloudflare run-time when the “implicit response” is invoked. The “implicit response” is my name for it. The cloudflare documentation calls it “the runtime proxies the request to the origin”.

Another way to describe ‘the implicit response’ is: “if respondWith() isn’t called during the sync phase of any event listener for a fetch event, then the request is handled as if the worker was never there. The passing of the request out of the worker and to the “origin” happens silently (silence=bad). But, the worker will still run, until the Response from the origin is resolved, which usually takes around 30ms+. And if you call respondWith() after this point (ie. from within any async context), then respondWith() will fail. Again silently. But. As you get zero error messages/warnings from this ‘too late’ respondWith() and the implicit response invocation, and as the worker is running for 30ms+ doing stuff, how can you the developer know what is wrong?”

Or something like that. I don’t fully know. That is why I would like some more documentation/discussion about the “implied response”. Or what it is called.

Hence. I want 1 warning! and 1 error! I want 1 warning in the console when the implicit response is activated. And I want 1 error when I call respondWith() too late.

ps. the polyfix uses another cryptic js trick: Promise.resolve().then(microtask). This essentially puts a task in the microtask queue. And for this issue, the microtask queue is a good fit for making the changes to the fetchEvent.respondWith() method to dispatch an Error. A simpler way to illustrate only the error message is:

addEventListener('fetch', async e => {
  try{
    Promise.resolve().then(()=>{
      e.respondWith = function(){throw new Error('respondWith() must be called sync');}
    });
    await new Promise(r=>setTimeout(r, 0));
    e.respondWith(new Response('this is too late, and you will get an error'));
  } catch(err){
    console.error(err);
  }
});

I stumbled across the not-so-intuitive “InvalidStateError: Too late to call FetchEvent.respondWith(). It must be called synchronously in the event handler.” Error. Adding a comment about it for completeness.

The only way I manage to get this Error, is by calling respondWith() with twice:

addEventListener('fetch', event => {
  event.respondWith(handleRequest(event.request, event))
})

/**
 * Fetch and log a given request object
 * @param {Request} request
 */
async function handleRequest(request, e) {
  console.log('Got request', request)
  const response = await fetch(request)
  console.log('Got response', response);
  try{
    e.respondWith(new Response('This is the second time we call "respondWith()".'));
  } catch(error){
    console.log(error.toString());
  }
  return response
}

This Error is only thrown when respondWith() is also called on the top sync scope, because if it isn’t, then the Implicit Response will kick in, and then you get no “too late” Error message. So, even though it is too late because respondWith() must be called from a sync scope, it is even more too late because your event listener has already called respondWith() from its sync scope. So, it would be more correct to see the other Error message here: “InvalidStateError: FetchEvent.respondWith() has already been called; it can only be called once.”

Second, it is nice to see that Cloudflare platform agrees with my desire to get an Error message when respondWith() is called too late/not sync.

Can anyone tell me where we post bug reports to the workers platform?