[`[bun hot]`] More reliability improvements to macOS watcher

3 files changed, 148 insertions, 30 deletions

diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig
index ccde433be..7ef309b13 100644
--- a/src/bun.js/javascript.zig
+++ b/src/bun.js/javascript.zig
@@ -2776,8 +2776,18 @@ pub const HotReloader = struct {
                     if (this.verbose)
                         Output.prettyErrorln("<r><d>File changed: {s}<r>", .{fs.relativeTo(file_path)});
 
-                    if (event.op.write) {
-                        current_task.append(id);
+                    if (comptime Environment.isMac) {
+                        // kqueue will report one of these if you rename, delete, or write to a file descriptor
+                        // since it's a file descriptor and not a file path, the
+                        // file descriptor will need to be closed for some
+                        // changes to be reflected
+                        if (event.op.write or event.op.delete or event.op.rename) {
+                            current_task.append(id);
+                        }
+                    } else {
+                        if (event.op.write) {
+                            current_task.append(id);
+                        }
                     }
                 },
                 .directory => {
@@ -2789,11 +2799,20 @@ pub const HotReloader = struct {
                             entries_option = rfs.entries.get(file_path);
                             var affected_i: usize = 0;
 
-                            if ((event.op.write or event.op.rename or event.op.delete) and entries_option != null) {
+                            // if a file descriptor is stale, we need to close it
+                            if (event.op.delete and entries_option != null) {
                                 for (parents) |parent_hash, entry_id| {
                                     if (parent_hash == id) {
-                                        affected_buf[affected_i] = file_paths[entry_id][file_path.len..];
+                                        const affected_path = file_paths[entry_id];
+                                        const was_deleted = check: {
+                                            std.os.access(affected_path, std.os.F_OK) catch break :check true;
+                                            break :check false;
+                                        };
+                                        if (!was_deleted) continue;
+
+                                        affected_buf[affected_i] = affected_path[file_path.len..];
                                         affected_i += 1;
+                                        if (affected_i >= affected_buf.len) break;
                                     }
                                 }
                             }
diff --git a/src/watcher.zig b/src/watcher.zig
index 7d610088d..e51dad72e 100644
--- a/src/watcher.zig
+++ b/src/watcher.zig
@@ -293,7 +293,7 @@ pub const WatchEvent = struct {
             .op = Op{
                 .delete = (kevent.fflags & std.c.NOTE_DELETE) > 0,
                 .metadata = (kevent.fflags & std.c.NOTE_ATTRIB) > 0,
-                .rename = (kevent.fflags & std.c.NOTE_RENAME) > 0,
+                .rename = (kevent.fflags & (std.c.NOTE_RENAME | std.c.NOTE_LINK)) > 0,
                 .write = (kevent.fflags & std.c.NOTE_WRITE) > 0,
             },
             .index = @truncate(WatchItemIndex, kevent.udata),
@@ -463,7 +463,7 @@ pub fn NewWatcher(comptime ContextType: type) type {
                 while (true) {
                     defer Output.flush();
 
-                    const count_ = std.os.system.kevent(
+                    var count_ = std.os.system.kevent(
                         DarwinWatcher.fd,
                         @as([*]KEvent, changelist),
                         0,
@@ -473,10 +473,44 @@ pub fn NewWatcher(comptime ContextType: type) type {
                         null,
                     );
 
+                    // Give the events more time to coallesce
+                    if (count_ < 128 / 2) {
+                        const remain = 128 - count_;
+                        var timespec = std.os.timespec{ .tv_sec = 0, .tv_nsec = 100_000 };
+                        const extra = std.os.system.kevent(
+                            DarwinWatcher.fd,
+                            @as([*]KEvent, changelist[@intCast(usize, count_)..].ptr),
+                            0,
+                            @as([*]KEvent, changelist[@intCast(usize, count_)..].ptr),
+                            remain,
+
+                            &timespec,
+                        );
+
+                        count_ += extra;
+                    }
+
                     var changes = changelist[0..@intCast(usize, @max(0, count_))];
                     var watchevents = this.watch_events[0..changes.len];
-                    for (changes) |event, i| {
-                        watchevents[i].fromKEvent(event);
+                    var out_len: usize = 0;
+                    if (changes.len > 0) {
+                        watchevents[0].fromKEvent(changes[0]);
+                        out_len = 1;
+                        var prev_event = changes[0];
+                        for (changes[1..]) |event| {
+                            if (prev_event.udata == event.udata) {
+                                var new: WatchEvent = undefined;
+                                new.fromKEvent(event);
+                                watchevents[out_len - 1].merge(new);
+                                continue;
+                            }
+
+                            watchevents[out_len].fromKEvent(event);
+                            prev_event = event;
+                            out_len += 1;
+                        }
+
+                        watchevents = watchevents[0..out_len];
                     }
 
                     this.mutex.lock();
@@ -605,7 +639,7 @@ pub fn NewWatcher(comptime ContextType: type) type {
             package_json: ?*PackageJSON,
             comptime copy_file_path: bool,
         ) !void {
-            var index: PlatformWatcher.EventListIndex = undefined;
+            var index: PlatformWatcher.EventListIndex = std.math.maxInt(PlatformWatcher.EventListIndex);
             const watchlist_id = this.watchlist.len;
 
             const file_path_: string = if (comptime copy_file_path)
@@ -615,7 +649,6 @@ pub fn NewWatcher(comptime ContextType: type) type {
 
             if (comptime Environment.isMac) {
                 const KEvent = std.c.Kevent;
-                index = DarwinWatcher.eventlist_index;
 
                 // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/kqueue.2.html
                 var event = std.mem.zeroes(KEvent);
@@ -629,11 +662,9 @@ pub fn NewWatcher(comptime ContextType: type) type {
                 // id
                 event.ident = @intCast(usize, fd);
 
-                DarwinWatcher.eventlist_index += 1;
-
                 // Store the hash for fast filtering later
                 event.udata = @intCast(usize, watchlist_id);
-                DarwinWatcher.eventlist[index] = event;
+                var events: [1]KEvent = .{event};
 
                 // This took a lot of work to figure out the right permutation
                 // Basically:
@@ -641,9 +672,9 @@ pub fn NewWatcher(comptime ContextType: type) type {
                 // our while(true) loop above receives notification of changes to any of the events created here.
                 _ = std.os.system.kevent(
                     DarwinWatcher.fd,
-                    DarwinWatcher.eventlist[index .. index + 1].ptr,
+                    @as([]KEvent, events[0..1]).ptr,
                     1,
-                    DarwinWatcher.eventlist[index .. index + 1].ptr,
+                    @as([]KEvent, events[0..1]).ptr,
                     0,
                     null,
                 );
@@ -685,7 +716,7 @@ pub fn NewWatcher(comptime ContextType: type) type {
             };
 
             const parent_hash = Watcher.getHash(Fs.PathName.init(file_path).dirWithTrailingSlash());
-            var index: PlatformWatcher.EventListIndex = undefined;
+            var index: PlatformWatcher.EventListIndex = std.math.maxInt(PlatformWatcher.EventListIndex);
 
             const file_path_: string = if (comptime copy_file_path)
                 std.mem.span(try this.allocator.dupeZ(u8, file_path))
@@ -695,7 +726,6 @@ pub fn NewWatcher(comptime ContextType: type) type {
             const watchlist_id = this.watchlist.len;
 
             if (Environment.isMac) {
-                index = DarwinWatcher.eventlist_index;
                 const KEvent = std.c.Kevent;
 
                 // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/kqueue.2.html
@@ -714,10 +744,9 @@ pub fn NewWatcher(comptime ContextType: type) type {
                 // id
                 event.ident = @intCast(usize, fd);
 
-                DarwinWatcher.eventlist_index += 1;
                 // Store the hash for fast filtering later
                 event.udata = @intCast(usize, watchlist_id);
-                DarwinWatcher.eventlist[index] = event;
+                var events: [1]KEvent = .{event};
 
                 // This took a lot of work to figure out the right permutation
                 // Basically:
@@ -725,9 +754,9 @@ pub fn NewWatcher(comptime ContextType: type) type {
                 // our while(true) loop above receives notification of changes to any of the events created here.
                 _ = std.os.system.kevent(
                     DarwinWatcher.fd,
-                    DarwinWatcher.eventlist[index .. index + 1].ptr,
+                    @as([]KEvent, events[0..1]).ptr,
                     1,
-                    DarwinWatcher.eventlist[index .. index + 1].ptr,
+                    @as([]KEvent, events[0..1]).ptr,
                     0,
                     null,
                 );
diff --git a/test/bun.js/hot.test.ts b/test/bun.js/hot.test.ts
index c7c8d0524..97dff6d91 100644
--- a/test/bun.js/hot.test.ts
+++ b/test/bun.js/hot.test.ts
@@ -39,7 +39,7 @@ it("should hot reload when file is overwritten", async () => {
         break;
       }
 
-      expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+      expect(line).toContain(`[#!root] Reloaded: ${reloadCounter}`);
       any = true;
     }
 
@@ -49,6 +49,73 @@ it("should hot reload when file is overwritten", async () => {
   expect(reloadCounter).toBe(3);
 });
 
+it("should recover from errors", async () => {
+  const root = import.meta.dir + "/hot-runner.js";
+  const runner = spawn({
+    cmd: [bunExe(), "--hot", "run", root],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+    stdin: "ignore",
+  });
+
+  let reloadCounter = 0;
+  const input = readFileSync(root, "utf-8");
+  function onReloadGood() {
+    writeFileSync(root, input);
+  }
+
+  function onReloadError() {
+    writeFileSync(root, "throw new Error('error');\n");
+  }
+
+  var queue = [onReloadError, onReloadGood, onReloadError, onReloadGood];
+  var errors: string[] = [];
+  var onError;
+  (async () => {
+    for await (let line of runner.stderr!) {
+      var str = new TextDecoder().decode(line);
+      errors.push(str);
+      onError && onError(str);
+    }
+  })();
+
+  for await (const line of runner.stdout!) {
+    var str = new TextDecoder().decode(line);
+    var any = false;
+    for (let line of str.split("\n")) {
+      if (!line.includes("[#!root]")) continue;
+      reloadCounter++;
+
+      if (reloadCounter === 3) {
+        runner.unref();
+        runner.kill();
+        break;
+      }
+
+      expect(line).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+      any = true;
+    }
+
+    if (any) {
+      queue.shift()!();
+      await new Promise<void>((resolve, reject) => {
+        if (errors.length > 0) {
+          errors.length = 0;
+          resolve();
+          return;
+        }
+
+        onError = resolve;
+      });
+
+      queue.shift()!();
+    }
+  }
+
+  expect(reloadCounter).toBe(3);
+});
+
 it("should not hot reload when a random file is written", async () => {
   const root = import.meta.dir + "/hot-runner.js";
   const runner = spawn({
@@ -59,16 +126,16 @@ it("should not hot reload when a random file is written", async () => {
     stdin: "ignore",
   });
 
-  var reloadCounter = 0;
-
+  let reloadCounter = 0;
+  const code = readFileSync(root, "utf-8");
   async function onReload() {
-    writeFileSync(root + ".another.yet.js", readFileSync(root, "utf-8"));
+    writeFileSync(root + ".another.yet.js", code);
     unlinkSync(root + ".another.yet.js");
   }
   var waiter = new Promise<void>((resolve, reject) => {
     setTimeout(async () => {
       resolve();
-    }, 10);
+    }, 50);
   });
   var finished = false;
   await Promise.race([
@@ -85,10 +152,13 @@ it("should not hot reload when a random file is written", async () => {
         var str = new TextDecoder().decode(line);
         for (let line of str.split("\n")) {
           if (!line.includes("[#!root]")) continue;
+          if (finished) {
+            return;
+          }
           await onReload();
 
           reloadCounter++;
-          expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+          expect(line).toContain(`[#!root] Reloaded: ${reloadCounter}`);
         }
       }
     })(),
@@ -97,7 +167,7 @@ it("should not hot reload when a random file is written", async () => {
   runner.kill(0);
   runner.unref();
 
-  expect(reloadCounter).toBe(0);
+  expect(reloadCounter).toBe(1);
 });
 
 it("should hot reload when a file is deleted and rewritten", async () => {
@@ -131,7 +201,7 @@ it("should hot reload when a file is deleted and rewritten", async () => {
         break;
       }
 
-      expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+      expect(line).toContain(`[#!root] Reloaded: ${reloadCounter}`);
       any = true;
     }
 
@@ -176,7 +246,7 @@ it("should hot reload when a file is renamed() into place", async () => {
         break;
       }
 
-      expect(str).toContain(`[#!root] Reloaded: ${reloadCounter}`);
+      expect(line).toContain(`[#!root] Reloaded: ${reloadCounter}`);
       any = true;
     }