Merge branch 'cloudflare:main' into main

src/node/README.md

@@ -19,3 +19,5 @@
 1. We will seek to prioritize implementation of Node.js APIs that are needed by npm modules and other ecosystem dependencies that our customers want and are trying to use. When those modules require Node.js core APIs, we will implement those in accordance with this framework. In some cases that may mean it will not be possible to support the module as written. Like the Node.js APIs themselves, support will be best-effort. When we cannot fully implement support for these modules directly in the runtime, we will attempt to work with the maintainers of those modules to implement a workers-compatible alternative or work to develop alternative workarounds.
 
 1. Polyfills of Node.js APIs (that is, external implementations that are not bundled directly into the workers runtime) may be leveraged as a last-resort alternative to patch over parts of the Node.js API we choose not to implement in the runtime. When used, these must be generally available for any Workers user, not only those using wrangler. The built-in implementation of Node.js APIs should always take precedence in general but individual workers should be able to BYOI ("bring your own implementation") within the reasonable constraints of the runtime. Cloudflare tooling should never polyfill an API that already exists within the runtime, and the existence of a polyfill implementation will not rule out implementing the API directly in the runtime.
+
+1. Experimental APIs only recently added to Node.js should not be implemented immediately in the workers runtime. Such APIs may be unstable for some time and could cause long term backwards compatibility issues or other unfortunate complexities in the workers runtime due to our "No Breaking Changes Without Compatibility Flags" policy. It is better to allow these APIs to sit and mature a bit in the Node.js runtime to ensure they are stable before being implemented in the workers runtime. Exceptions can be made to address immediate priority use cases.

src/node/internal/internal_timers.ts

@@ -24,6 +24,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 import { validateFunction } from 'node-internal:validators';
+import { default as timersUtil } from 'node-internal:timers';
 
 let clearTimeoutImpl: (obj: Timeout) => void;
 
@@ -134,9 +135,9 @@ export function clearTimeout(timer: unknown): void {
   }
 }
 
-export const setImmediate = globalThis.setImmediate.bind(globalThis);
+export const setImmediate = timersUtil.setImmediate.bind(timersUtil);
 
-export const clearImmediate = globalThis.clearImmediate.bind(globalThis);
+export const clearImmediate = timersUtil.clearImmediate.bind(timersUtil);
 
 export function setInterval(
   callback: (...args: unknown[]) => void,

src/node/internal/internal_timers_promises.ts

@@ -102,13 +102,13 @@ export async function setImmediate<T>(
 
   const { promise, resolve, reject } = Promise.withResolvers<T>();
 
-  const timer = globalThis.setImmediate(() => {
+  const timer = timers.setImmediate(() => {
     resolve(value as T);
   });
 
   if (signal) {
     function onCancel(): void {
-      globalThis.clearImmediate(timer);
+      timers.clearImmediate(timer);
       signal?.removeEventListener('abort', onCancel);
       reject(new AbortError(undefined, { cause: signal?.reason }));
     }

src/node/internal/timers.d.ts

@@ -0,0 +1,7 @@
+import type {
+  setImmediate as setImmediateImpl,
+  clearImmediate as clearImmediateImpl,
+} from 'node:timers';
+
+export const setImmediate: typeof setImmediateImpl;
+export const clearImmediate: typeof clearImmediateImpl;

src/node/internal/util.d.ts

@@ -121,6 +121,6 @@ export function isBoxedPrimitive(
 ): value is number | string | boolean | bigint | symbol;
 
 export function getBuiltinModule(id: string): any;
-export function getCallSite(frames: number): Record<string, string>[];
+export function getCallSites(frames?: number): Record<string, string>[];
 export function processExitImpl(code: number): void;
 export const processPlatform: string;

src/node/util.ts

@@ -206,9 +206,12 @@ export function deprecate(
   return fn;
 }
 
-export function getCallSite(frames: number = 10) {
-  return utilImpl.getCallSite(frames);
-}
+// Node.js originally introduced the API with the name `getCallSite()` as an experimental
+// API but then renamed it to `getCallSites()` soon after. We had already implemented the
+// API with the original name in a release. To avoid the possibility of breaking, we export
+// the function using both names.
+export const getCallSite = utilImpl.getCallSites.bind(utilImpl);
+export const getCallSites = utilImpl.getCallSites.bind(utilImpl);
 
 export function isDeepStrictEqual(a: unknown, b: unknown): boolean {
   return _isDeepStrictEqual(a, b);
@@ -247,6 +250,7 @@ export default {
   transferableAbortController,
   transferableAbortSignal,
   getCallSite,
+  getCallSites,
   isDeepStrictEqual,
   isArray,
 };

src/workerd/api/node/BUILD.bazel

@@ -10,13 +10,15 @@ wd_cc_library(
         "crypto-keys.c++",
         "diagnostics-channel.c++",
         "dns.c++",
+        "timers.c++",
         "zlib-util.c++",
     ],
     hdrs = [
         "crypto.h",
         "diagnostics-channel.h",
         "dns.h",
         "node.h",
+        "timers.h",
         "zlib-util.h",
     ],
     implementation_deps = [

src/workerd/api/node/node.h

@@ -8,6 +8,7 @@
 #include <workerd/api/node/buffer.h>
 #include <workerd/api/node/dns.h>
 #include <workerd/api/node/module.h>
+#include <workerd/api/node/timers.h>
 #include <workerd/api/node/url.h>
 #include <workerd/api/node/util.h>
 #include <workerd/io/compatibility-date.h>
@@ -52,7 +53,8 @@ class CompatibilityFlags: public jsg::Object {
   V(DiagnosticsChannelModule, "node-internal:diagnostics_channel")                                 \
   V(ZlibUtil, "node-internal:zlib")                                                                \
   V(UrlUtil, "node-internal:url")                                                                  \
-  V(DnsUtil, "node-internal:dns")
+  V(DnsUtil, "node-internal:dns")                                                                  \
+  V(TimersUtil, "node-internal:timers")
 
 // Add to the NODEJS_MODULES_EXPERIMENTAL list any currently in-development
 // node.js compat C++ modules that should be guarded by the experimental compat
@@ -144,4 +146,4 @@ kj::Own<jsg::modules::ModuleBundle> getExternalNodeJsCompatModuleBundle(auto fea
   api::node::CompatibilityFlags, EW_NODE_BUFFER_ISOLATE_TYPES, EW_NODE_CRYPTO_ISOLATE_TYPES,       \
       EW_NODE_DIAGNOSTICCHANNEL_ISOLATE_TYPES, EW_NODE_ASYNCHOOKS_ISOLATE_TYPES,                   \
       EW_NODE_UTIL_ISOLATE_TYPES, EW_NODE_ZLIB_ISOLATE_TYPES, EW_NODE_URL_ISOLATE_TYPES,           \
-      EW_NODE_MODULE_ISOLATE_TYPES, EW_NODE_DNS_ISOLATE_TYPES\
+      EW_NODE_MODULE_ISOLATE_TYPES, EW_NODE_DNS_ISOLATE_TYPES, EW_NODE_TIMERS_ISOLATE_TYPES\

src/workerd/api/node/tests/timers-nodejs-test.js

@@ -53,7 +53,7 @@ export const testSetImmediate = {
 
     {
       const { promise, resolve } = Promise.withResolvers();
-      globalThis.setImmediate(
+      timers.setImmediate(
         (...args) => {
           deepStrictEqual(args, [1, 2, 3]);
           resolve();

src/workerd/api/node/tests/timers-nodejs-test.wd-test

@@ -8,7 +8,7 @@ const unitTests :Workerd.Config = (
           (name = "worker", esModule = embed "timers-nodejs-test.js")
         ],
         compatibilityDate = "2025-01-09",
-        compatibilityFlags = ["nodejs_compat"],
+        compatibilityFlags = ["nodejs_compat", "no_nodejs_compat_v2"],
       )
     ),
   ],

src/workerd/api/node/tests/util-nodejs-test.js

@@ -4343,15 +4343,23 @@ export const debuglog = {
   },
 };
 
-export const getCallSiteTest = {
+export const getCallSitesTest = {
   test() {
-    const callSites = util.getCallSite();
+    const callSites = util.getCallSites();
     assert.strictEqual(callSites.length, 1);
     const [stack] = callSites;
     assert.strictEqual(stack.functionName, 'test');
     assert.strictEqual(stack.scriptName, 'worker');
     assert.strictEqual(typeof stack.lineNumber, 'number');
+    assert.strictEqual(typeof stack.columnNumber, 'number');
     assert.strictEqual(typeof stack.column, 'number');
+
+    // We leave this implementation for compat reasons.
+    // Node.js originally introduced the API with the name `getCallSite()` as an experimental
+    // API but then renamed it to `getCallSites()` soon after. We had already implemented the
+    // API with the original name in a release. To avoid the possibility of breaking, we export
+    // the function using both names.
+    assert.strictEqual(typeof util.getCallSite, 'function');
   },
 };
 

src/workerd/api/node/timers.c++

@@ -0,0 +1,27 @@
+#include "timers.h"
+
+#include <workerd/api/global-scope.h>
+#include <workerd/jsg/jsg.h>
+
+namespace workerd::api::node {
+
+// The setImmediate/clearImmediate methods are only exposed on globalThis if the
+// node_compat_v2 flag is set. However, we want them exposed via `node:timers`
+// generally when just the original node_compat is enabled. Therefore, we provide
+// this alternative route to the implementations on ServiceWorkerGlobalScope.
+jsg::Ref<Immediate> TimersUtil::setImmediate(jsg::Lock& js,
+    jsg::Function<void(jsg::Arguments<jsg::Value>)> function,
+    jsg::Arguments<jsg::Value> args) {
+  auto context = js.v8Context();
+  auto& global =
+      jsg::extractInternalPointer<ServiceWorkerGlobalScope, true>(context, context->Global());
+  return global.setImmediate(js, kj::mv(function), kj::mv(args));
+}
+
+void TimersUtil::clearImmediate(jsg::Lock& js, kj::Maybe<jsg::Ref<Immediate>> maybeImmediate) {
+  auto context = js.v8Context();
+  auto& global =
+      jsg::extractInternalPointer<ServiceWorkerGlobalScope, true>(context, context->Global());
+  global.clearImmediate(kj::mv(maybeImmediate));
+}
+}  // namespace workerd::api::node

src/workerd/api/node/timers.h

@@ -0,0 +1,34 @@
+// Copyright (c) 2017-2022 Cloudflare, Inc.
+// Licensed under the Apache 2.0 license found in the LICENSE file or at:
+//     https://opensource.org/licenses/Apache-2.0
+#pragma once
+
+#include <workerd/jsg/jsg.h>
+
+#include <kj/string.h>
+
+namespace workerd::api {
+
+class Immediate;
+
+namespace node {
+class TimersUtil final: public jsg::Object {
+ public:
+  TimersUtil() = default;
+  TimersUtil(jsg::Lock&, const jsg::Url&) {}
+
+  jsg::Ref<Immediate> setImmediate(jsg::Lock& js,
+      jsg::Function<void(jsg::Arguments<jsg::Value>)> function,
+      jsg::Arguments<jsg::Value> args);
+  void clearImmediate(jsg::Lock& js, kj::Maybe<jsg::Ref<Immediate>> immediate);
+
+  JSG_RESOURCE_TYPE(TimersUtil) {
+    JSG_METHOD(setImmediate);
+    JSG_METHOD(clearImmediate);
+  }
+};
+
+#define EW_NODE_TIMERS_ISOLATE_TYPES api::node::TimersUtil
+}  // namespace node
+
+}  // namespace workerd::api

src/workerd/api/node/util.c++

@@ -177,22 +177,29 @@ jsg::Name UtilModule::getResourceTypeInspect(jsg::Lock& js) {
   return js.newApiSymbol("kResourceTypeInspect"_kj);
 }
 
-kj::Array<UtilModule::CallSiteEntry> UtilModule::getCallSite(jsg::Lock& js, int frames) {
-  JSG_REQUIRE(
-      frames >= 1 && frames <= 200, Error, "Frame count should be between 1 and 200 inclusive."_kj);
-  auto stack = v8::StackTrace::CurrentStackTrace(js.v8Isolate, frames + 1);
+kj::Array<UtilModule::CallSiteEntry> UtilModule::getCallSites(
+    jsg::Lock& js, jsg::Optional<int> frames) {
+  KJ_IF_SOME(f, frames) {
+    JSG_REQUIRE(f >= 1 && f <= 200, Error, "Frame count should be between 1 and 200 inclusive."_kj);
+  }
+
+  auto stack = v8::StackTrace::CurrentStackTrace(js.v8Isolate, frames.orDefault(10) + 1);
   const int frameCount = stack->GetFrameCount();
   auto objects = kj::Vector<CallSiteEntry>();
   objects.reserve(frameCount - 1);
 
-  // Frame 0 is node:util. It should be skipped.
-  for (int i = 1; i < frameCount; ++i) {
+  for (int i = 0; i < frameCount; ++i) {
     auto stack_frame = stack->GetFrame(js.v8Isolate, i);
 
     objects.add(CallSiteEntry{
       .functionName = js.toString(stack_frame->GetFunctionName()),
       .scriptName = js.toString(stack_frame->GetScriptName()),
       .lineNumber = stack_frame->GetLineNumber(),
+      // Node.js originally implemented the experimental API using the "column" field
+      // then later renamed it to columnNumber. We had already implemented the API
+      // using column. To ensure backwards compat without the complexity of a compat
+      // flag, we just export both.
+      .columnNumber = stack_frame->GetColumn(),
       .column = stack_frame->GetColumn(),
     });
   }

src/workerd/api/node/util.h

@@ -212,11 +212,16 @@ class UtilModule final: public jsg::Object {
     kj::String functionName;
     kj::String scriptName;
     int lineNumber;
+    // Node.js originally introduced the API with the name `getCallSite()` as an experimental
+    // API but then renamed it to `getCallSites()` soon after. We had already implemented the
+    // API with the original name in a release. To avoid the possibility of breaking, we export
+    // the function using both names.
+    int columnNumber;
     int column;
 
-    JSG_STRUCT(functionName, scriptName, lineNumber, column);
+    JSG_STRUCT(functionName, scriptName, lineNumber, columnNumber, column);
   };
-  kj::Array<CallSiteEntry> getCallSite(jsg::Lock& js, int frames);
+  kj::Array<CallSiteEntry> getCallSites(jsg::Lock& js, jsg::Optional<int> frames);
 
 #define V(Type) bool is##Type(jsg::JsValue value);
   JS_UTIL_IS_TYPES(V)
@@ -257,7 +262,7 @@ class UtilModule final: public jsg::Object {
     JSG_METHOD(getProxyDetails);
     JSG_METHOD(previewEntries);
     JSG_METHOD(getConstructorName);
-    JSG_METHOD(getCallSite);
+    JSG_METHOD(getCallSites);
 
 #define V(Type) JSG_METHOD(is##Type);
     JS_UTIL_IS_TYPES(V)

src/workerd/io/actor-sqlite.c++

@@ -128,7 +128,7 @@ ActorSqlite::ExplicitTxn::~ExplicitTxn() noexcept(false) {
     }
   }();
 
-  if (!committed) {
+  if (!committed && actorSqlite.broken == kj::none) {
     // Assume rollback if not committed.
     rollbackImpl();
   }