cloudflare / cloudflare/workerd
`setTimeout` and `fetch` never callback/fulfill when DO gets evicted
- Dominant language
- C++
- Stars
- 8.7k
- Forks
- 739
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 174
Description
`fetch` returns a promise that should eventually be fulfilled (by either resolve or reject). When a DO is evicted, this promise never fulfills.
Often this is fine because the execution environment is destroyed, so the fact that the fetch never fulfills is not observable. But in the case where the execution environment is reused, ie because a new DO is instantiated within it, application code can observe that `fetch` never fulfills.
A similar thing happens with `setTimeout` and `setInterval`. If you set a timer from a DO and the DO is evicted, the callback for the timer is not called. Normally this is fine because the execution environment goes away. But in the case where execution environment is reused, it can lead to weird results.
One example we have run into where this hurt us:
- We have a log sink in a DO that we want to flush (by sending logs to a server) periodically
- We set a periodic timer to send the logs
- Somebody accidentally makes this sink global
- When a DO is unloaded, the timer stops getting called
- We stop sending logs
- Eventually the log buffer fills up and the program crashes
- We don't have any logs prior to the crash because they weren't getting sent
Here is a GitHub repo demonstrating the above issue:
https://github.com/rocicorp/cf-global-state/blob/main/README.md
We are aware of the advise to not use global variables and do attempt to follow it. However they have crept into our worker code a number of times by accident, frequently with this kind of pretty serious problem. It is difficult to completely avoid globals in larger code bases, since they are part of the javascript ecosystem.
I think that if JS contexts are going to get reused, then the platform should ensure any waiting promises and callbacks do get fired. If the promises are going to be abandoned, then the js context should also be killed so that it's not detectable. Otherwise the behavior seems inconsistent with the contract of these APIs.
Another solution that would work for us would be to provide an option to always load DOs into a new `v8::Context`. This would slightly slow down the launch of DOs, but would completely avoid the problem with global variables since global code would re-run at the start of each DO instantiation.
Contributor guide
Assessment
This issue has not been assessed yet.