More tests

src/bun.js/bindings/JSBundlerPlugin.cpp

@@ -284,8 +284,11 @@ int BundlerPlugin::NativePluginList::call(JSC::VM& vm, int* shouldContinue, void
         {
             std::lock_guard<std::mutex> lock(*group->at(i).second);
             if (group->at(i).first.match(path) > -1) {
-                ((OnBeforeParseArgs*) (onBeforeParseArgs))->external = callbacks[i].second->value();
-                callbacks[i].first(1, onBeforeParseArgs, onBeforeParseResult);
+                Bun::NapiExternal* external = callbacks[i].second;
+                if (external) {
+                    ((OnBeforeParseArgs*) (onBeforeParseArgs))->external = external->value();
+                }
+                callbacks[i].first(onBeforeParseArgs, onBeforeParseResult);
                 count++;
             }
         }
@@ -306,6 +309,9 @@ JSC_DEFINE_HOST_FUNCTION(jsBundlerPluginFunction_onBeforeParse, (JSC::JSGlobalOb
         return JSC::JSValue::encode(JSC::jsUndefined());
     }
 
+    // Clone the regexp so we don't have to worry about it being used concurrently with the JS thread.
+    // TODO: Should we have a regexp object for every thread in the thread pool? Then we could avoid using
+    // a mutex to synchronize access to the same regexp from multiple threads.
     JSC::RegExpObject* jsRegexp = jsCast<JSC::RegExpObject*>(callFrame->argument(0));
     RegExp* reggie = jsRegexp->regExp();
     RegExp* newRegexp = RegExp::create(vm, reggie->pattern(), reggie->flags());

src/bun.js/bindings/JSBundlerPlugin.h

@@ -10,7 +10,7 @@
 typedef void (*JSBundlerPluginAddErrorCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue);
 typedef void (*JSBundlerPluginOnLoadAsyncCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue);
 typedef void (*JSBundlerPluginOnResolveAsyncCallback)(void*, void*, JSC::EncodedJSValue, JSC::EncodedJSValue, JSC::EncodedJSValue);
-typedef void (*JSBundlerPluginNativeOnBeforeParseCallback)(int, void*, void*);
+typedef void (*JSBundlerPluginNativeOnBeforeParseCallback)( void*, void*);
 
 namespace Bun {
 
@@ -47,6 +47,7 @@ public:
     };
 
     typedef struct {
+        size_t __struct_size;
         void *context;
         const char *path_ptr;
         size_t path_len;

src/bundler/bundle_v2.zig

@@ -3818,6 +3818,7 @@ pub const ParseTask = struct {
         result: ?*OnBeforeParseResult = null,
 
         const OnBeforeParseArguments = extern struct {
+            struct_size: usize = @sizeOf(OnBeforeParseArguments),
             context: *OnBeforeParsePlugin,
             path_ptr: [*]const u8 = "",
             path_len: usize = 0,
@@ -3828,6 +3829,7 @@ pub const ParseTask = struct {
         };
 
         const BunLogOptions = extern struct {
+            struct_size: usize = @sizeOf(BunLogOptions),
             message_ptr: ?[*]const u8 = null,
             message_len: usize = 0,
             path_ptr: ?[*]const u8 = null,
@@ -3906,6 +3908,7 @@ pub const ParseTask = struct {
         };
 
         const OnBeforeParseResult = extern struct {
+            struct_size: usize = @sizeOf(OnBeforeParseResult),
             source_ptr: ?[*]const u8 = null,
             source_len: usize = 0,
             loader: Loader,

test/js/bun/plugin/native-plugin.test.ts

@@ -1,5 +1,5 @@
 import { plugin } from "bun";
-import { afterEach, beforeAll, beforeEach, describe, expect, it, test } from "bun:test";
+import { afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test";
 import path, { dirname, join, resolve } from "path";
 import source from "./native_plugin.c" with { type: "file" };
 import { bunEnv, bunExe, tempDirWithFiles } from "harness";
@@ -50,6 +50,8 @@ console.log(JSON.stringify(json));`,
     tempdir = tempDirWithFiles("native-plugins", files);
     outdir = path.join(tempdir, "dist");
 
+    console.log("tempdir", tempdir);
+
     process.chdir(tempdir);
 
     await Bun.$`${bunExe()} i && ${bunExe()} build:napi`.env(bunEnv).cwd(tempdir);
@@ -60,7 +62,7 @@ console.log(JSON.stringify(json));`,
     process.chdir(cwd);
   });
 
-  test("basic", async () => {
+  it("works in a basic case", async () => {
     await Bun.$`${bunExe()} i && ${bunExe()} build:napi`.env(bunEnv).cwd(tempdir);
 
     const result = await Bun.build({
@@ -94,7 +96,7 @@ console.log(JSON.stringify(json));`,
     expect(output).toStrictEqual({ fooCount: 8 });
   });
 
-  test("doesn't explode when there are a lot of concurrent files", async () => {
+  it("doesn't explode when there are a lot of concurrent files", async () => {
     // Generate 100 json files
     const files: [filepath: string, var_name: string][] = await Promise.all(
       Array.from({ length: 100 }, async (_, i) => {
@@ -105,8 +107,8 @@ console.log(JSON.stringify(json));`,
 
     // Append the imports to index.ts
     const prelude = /* ts */ `import values from "./stuff.ts"
-const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
-    `;
+  const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
+      `;
     await Bun.$`echo ${prelude} > index.ts`;
     await Bun.$`echo ${files.map(([fp]) => fp).join("\n")} >> index.ts`;
     await Bun.$`echo ${files.map(([, varname]) => `console.log(JSON.stringify(${varname}))`).join("\n")} >> index.ts`;
@@ -147,4 +149,178 @@ const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
       expect(json).toStrictEqual({ fooCount: 8 });
     }
   });
+
+  // We clone the RegExp object in the C++ code so this test ensures that there
+  // is no funny business regarding the filter regular expression and multiple
+  // threads
+  it("doesn't explode when there are a lot of concurrent files AND the filter regex is used on the JS thread", async () => {
+    const filter = /\.ts/;
+    // Generate 100 json files
+    const files: [filepath: string, var_name: string][] = await Promise.all(
+      Array.from({ length: 100 }, async (_, i) => {
+        await Bun.write(path.join(tempdir, "json_files", `lmao${i}.json`), `{}`);
+        return [`import json${i} from "./json_files/lmao${i}.json"`, `json${i}`];
+      }),
+    );
+
+    // Append the imports to index.ts
+    const prelude = /* ts */ `import values from "./stuff.ts"
+const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
+    `;
+    await Bun.$`echo ${prelude} > index.ts`;
+    await Bun.$`echo ${files.map(([fp]) => fp).join("\n")} >> index.ts`;
+    await Bun.$`echo ${files.map(([, varname]) => `console.log(JSON.stringify(${varname}))`).join("\n")} >> index.ts`;
+
+    const resultPromise = Bun.build({
+      outdir,
+      entrypoints: [path.join(tempdir, "index.ts")],
+      plugins: [
+        {
+          name: "xXx123_foo_counter_321xXx",
+          setup(build) {
+            const napiModule = require(path.join(tempdir, "build/Release/xXx123_foo_counter_321xXx.node"));
+            const external = napiModule.createExternal();
+
+            build.onBeforeParse({ filter }, { napiModule, symbol: "plugin_impl", external });
+
+            build.onLoad({ filter: /\.json/ }, async ({ defer, path }) => {
+              await defer();
+              const count = napiModule.getFooCount(external);
+              return {
+                contents: JSON.stringify({ fooCount: count }),
+                loader: "json",
+              };
+            });
+          },
+        },
+      ],
+    });
+
+    // Now saturate this thread with uses of the filter regex to test that nothing bad happens
+    // when the JS thread and the bundler thread use regexes concurrently
+    let dummy = 0;
+    for (let i = 0; i < 10000; i++) {
+      // Match the filter regex on some dummy string
+      dummy += filter.test("foo") ? 1 : 0;
+    }
+
+    const result = await resultPromise;
+
+    if (!result.success) console.log(result);
+    expect(result.success).toBeTrue();
+    const output = await Bun.$`${bunExe()} run dist/index.js`.cwd(tempdir).text();
+    const outputJsons = output
+      .trim()
+      .split("\n")
+      .map(s => JSON.parse(s));
+    for (const json of outputJsons) {
+      expect(json).toStrictEqual({ fooCount: 8 });
+    }
+  });
+
+  it("doesn't explode when passing invalid external", async () => {
+    const filter = /\.ts/;
+    // Generate 100 json files
+    const files: [filepath: string, var_name: string][] = await Promise.all(
+      Array.from({ length: 100 }, async (_, i) => {
+        await Bun.write(path.join(tempdir, "json_files", `lmao${i}.json`), `{}`);
+        return [`import json${i} from "./json_files/lmao${i}.json"`, `json${i}`];
+      }),
+    );
+
+    // Append the imports to index.ts
+    const prelude = /* ts */ `import values from "./stuff.ts"
+const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
+    `;
+    await Bun.$`echo ${prelude} > index.ts`;
+    await Bun.$`echo ${files.map(([fp]) => fp).join("\n")} >> index.ts`;
+    await Bun.$`echo ${files.map(([, varname]) => `console.log(JSON.stringify(${varname}))`).join("\n")} >> index.ts`;
+
+    const resultPromise = Bun.build({
+      outdir,
+      entrypoints: [path.join(tempdir, "index.ts")],
+      plugins: [
+        {
+          name: "xXx123_foo_counter_321xXx",
+          setup(build) {
+            const napiModule = require(path.join(tempdir, "build/Release/xXx123_foo_counter_321xXx.node"));
+            const external = undefined;
+
+            build.onBeforeParse({ filter }, { napiModule, symbol: "plugin_impl", external });
+
+            build.onLoad({ filter: /\.json/ }, async ({ defer, path }) => {
+              await defer();
+              let count = 0;
+              try {
+                count = napiModule.getFooCount(external);
+              } catch (e) {}
+              return {
+                contents: JSON.stringify({ fooCount: count }),
+                loader: "json",
+              };
+            });
+          },
+        },
+      ],
+    });
+
+    const result = await resultPromise;
+
+    if (!result.success) console.log(result);
+    expect(result.success).toBeTrue();
+    const output = await Bun.$`${bunExe()} run dist/index.js`.cwd(tempdir).text();
+    const outputJsons = output
+      .trim()
+      .split("\n")
+      .map(s => JSON.parse(s));
+    for (const json of outputJsons) {
+      expect(json).toStrictEqual({ fooCount: 0 });
+    }
+  });
+
+  it("works when logging an error", async () => {
+    const filter = /\.ts/;
+
+    const prelude = /* ts */ `import values from "./stuff.ts"
+  const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
+      `;
+    await Bun.$`echo ${prelude} > index.ts`;
+
+    const resultPromise = Bun.build({
+      outdir,
+      entrypoints: [path.join(tempdir, "index.ts")],
+      plugins: [
+        {
+          name: "xXx123_foo_counter_321xXx",
+          setup(build) {
+            const napiModule = require(path.join(tempdir, "build/Release/xXx123_foo_counter_321xXx.node"));
+            const external = napiModule.createExternal();
+            napiModule.setThrowsErrors(external, true);
+
+            build.onBeforeParse({ filter }, { napiModule, symbol: "plugin_impl", external });
+
+            build.onLoad({ filter: /\.json/ }, async ({ defer, path }) => {
+              await defer();
+              let count = 0;
+              try {
+                count = napiModule.getFooCount(external);
+              } catch (e) {}
+              return {
+                contents: JSON.stringify({ fooCount: count }),
+                loader: "json",
+              };
+            });
+          },
+        },
+      ],
+    });
+
+    const result = await resultPromise;
+
+    if (result.success) console.log(result);
+    expect(result.success).toBeFalse();
+    const log = result.logs[0];
+    expect(log.message).toContain("Throwing an error");
+    expect(log.level).toBe("error");
+  });
 });

test/js/bun/plugin/native_plugin.c

@@ -12,9 +12,12 @@
 
 typedef struct {
     atomic_size_t foo_count;
+    // For testing logging error logic
+    atomic_bool throws_an_error;
 } External;
 
 typedef struct {
+    size_t __struct_size;
     void* bun;
     const uint8_t* path_ptr;
     size_t path_len;
@@ -27,6 +30,7 @@ typedef struct {
 typedef struct BunLogOptions BunLogOptions;
 
 typedef struct OnBeforeParseResult {
+    size_t __struct_size;
     uint8_t* source_ptr;
     size_t source_len;
     uint8_t loader;
@@ -39,7 +43,61 @@ typedef struct OnBeforeParseResult {
     void (*log)(const OnBeforeParseArguments* args, BunLogOptions* options);
 }  OnBeforeParseResult;
 
-void plugin_impl(int version, const OnBeforeParseArguments* args, OnBeforeParseResult* result) {
+typedef struct BunLogOptions {
+    size_t __struct_size;
+    const uint8_t* message_ptr;
+    size_t message_len;
+    const uint8_t* path_ptr;
+    size_t path_len;
+    const uint8_t* source_line_text_ptr;
+    size_t source_line_text_len;
+    int8_t level;
+    int line;
+    int lineEnd;
+    int column;
+    int columnEnd;
+} BunLogOptions;
+
+typedef enum {
+    BUN_LOG_LEVEL_VERBOSE = 0,
+    BUN_LOG_LEVEL_DEBUG = 1,
+    BUN_LOG_LEVEL_INFO = 2,
+    BUN_LOG_LEVEL_WARN = 3,
+    BUN_LOG_LEVEL_ERROR = 4,
+} BunLogLevel;
+
+
+void log_error(const OnBeforeParseArguments* args, const OnBeforeParseResult* result, BunLogLevel level, const char* message, size_t message_len) {
+    BunLogOptions options = (BunLogOptions) {
+        .message_ptr = (uint8_t*)message,
+        .message_len = message_len,
+        .path_ptr = args->path_ptr,
+        .path_len = args->path_len,
+        .source_line_text_ptr = NULL,
+        .source_line_text_len = 0,
+        .level = level,
+        .line = 0,
+        .lineEnd = 0,
+        .column = 0,
+        .columnEnd = 0,
+    };
+    (result->log)(args, &options);
+}
+
+void plugin_impl(const OnBeforeParseArguments* args, OnBeforeParseResult* result) {
+    // if (args->__struct_size < sizeof(OnBeforeParseArguments)) {
+    //     log_error(args, result, BUN_LOG_LEVEL_ERROR, "Invalid OnBeforeParseArguments struct size", sizeof("Invalid OnBeforeParseArguments struct size") - 1);
+    //     return;
+    // }
+
+    if (args->external) {
+        External* external = (External*)args->external;
+        if (atomic_load(&external->throws_an_error)) {
+            log_error(args, result, BUN_LOG_LEVEL_ERROR, "Throwing an error", sizeof("Throwing an error") - 1);
+            return;
+        }
+    }
+
     int fetch_result = result->fetchSourceCode(args, result);
     if (fetch_result != 0) {
         printf("FUCK\n");
@@ -74,8 +132,10 @@ void plugin_impl(int version, const OnBeforeParseArguments* args, OnBeforeParseR
                 cursor = strnstr((const char*) cursor, "foo", (size_t) (end - cursor));
             } else break;
         }
-        External *external = (External*)args->external;
-        atomic_fetch_add(&external->foo_count, foo_count);
+        if (args->external) {
+            External *external = (External*)args->external;
+            atomic_fetch_add(&external->foo_count, foo_count);
+        }
         result->source_ptr = (uint8_t*)new_source;
         result->source_len = result->source_len;
     } else {
@@ -87,7 +147,6 @@ void plugin_impl(int version, const OnBeforeParseArguments* args, OnBeforeParseR
 }
 
 void finalizer(napi_env env, void* data, void* hint) {
-    printf("FREEING EXTERNAL!\n");
     External* external = (External*)data;
     if (external != NULL) {
         free(external);
@@ -118,6 +177,41 @@ napi_value create_external(napi_env env, napi_callback_info info) {
     return result;
 }
 
+napi_value set_throws_errors(napi_env env, napi_callback_info info) {
+    napi_status status;
+    External* external;
+
+    size_t argc = 1;
+    napi_value args[1];
+    status = napi_get_cb_info(env, info, &argc, args, NULL, NULL);
+    if (status != napi_ok) {
+        napi_throw_error(env, NULL, "Failed to parse arguments");
+        return NULL;
+    }
+
+    if (argc < 1) {
+        napi_throw_error(env, NULL, "Wrong number of arguments");
+        return NULL;
+    }
+
+    status = napi_get_value_external(env, args[0], (void**)&external);
+    if (status != napi_ok) {
+        napi_throw_error(env, NULL, "Failed to get external");
+        return NULL;
+    }
+
+    bool throws;
+    status = napi_get_value_bool(env, args[0], &throws);
+    if (status != napi_ok) {
+        napi_throw_error(env, NULL, "Failed to get boolean value");
+        return NULL;
+    }
+
+    atomic_store(&external->throws_an_error, throws);
+
+    return NULL;
+}
+
 napi_value get_foo_count(napi_env env, napi_callback_info info) {
     napi_status status;
     External* external;
@@ -162,6 +256,7 @@ napi_value Init(napi_env env, napi_value exports) {
     napi_status status;
     napi_value fn_get_names;
     napi_value fn_create_external;
+    napi_value fn_set_throws_errors;
 
     // Register get_names function
     status = napi_create_function(env, NULL, 0, get_foo_count, NULL, &fn_get_names);
@@ -175,6 +270,18 @@ napi_value Init(napi_env env, napi_value exports) {
         return NULL;
     }
 
+    // Register set_throws_errors function  
+    status = napi_create_function(env, NULL, 0, set_throws_errors, NULL, &fn_set_throws_errors);
+    if (status != napi_ok) {
+        napi_throw_error(env, NULL, "Failed to create set_throws_errors function");
+        return NULL;
+    }
+    status = napi_set_named_property(env, exports, "setThrowsErrors", fn_set_throws_errors);
+    if (status != napi_ok) {
+        napi_throw_error(env, NULL, "Failed to add set_throws_errors function to exports");
+        return NULL;
+    }
+
     // Register create_external function  
     status = napi_create_function(env, NULL, 0, create_external, NULL, &fn_create_external);
     if (status != napi_ok) {