Fix type errors and remove potentially precarious uses of unreachable

oven-sh · Apr 14, 2024 · 2d7aec0 · 2d7aec0
1 parent d033c9f
commit 2d7aec0
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 24 deletions.
diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig
@@ -2123,7 +2123,7 @@ pub const Interpreter = struct {
                 return;
             }
 
-            unreachable;
+            @panic("Invalid child to Expansion, this indicates a bug in Bun. Please file a report on Github.");
         }
 
         fn onGlobWalkDone(this: *Expansion, task: *ShellGlobTask) void {
@@ -2742,7 +2742,7 @@ pub const Interpreter = struct {
                 return;
             }
 
-            unreachable;
+            @panic("Invalid child to Assigns expression, this indicates a bug in Bun. Please file a report on Github.");
         }
     };
 
@@ -3237,7 +3237,7 @@ pub const Interpreter = struct {
                         if (ptr == @as(usize, @intCast(child.ptr.repr._ptr))) break :brk i;
                     }
                 }
-                unreachable;
+                @panic("Invalid pipeline state");
             };
 
             log("pipeline child done {x} ({d}) i={d}", .{ @intFromPtr(this), exit_code, idx });
@@ -4263,6 +4263,7 @@ pub const Interpreter = struct {
                     (if (this.stdout) |*stdout| stdout.closed() else true) and
                     (if (this.stderr) |*stderr| stderr.closed() else true);
                 log("BufferedIOClosed(0x{x}) all_closed={any} stdin={any} stdout={any} stderr={any}", .{ @intFromPtr(this), ret, if (this.stdin) |stdin| stdin else true, if (this.stdout) |*stdout| stdout.closed() else true, if (this.stderr) |*stderr| stderr.closed() else true });
+                std.debug.print("all_closed={any} stdin={any} stdout={any} stderr={any}\n", .{ ret, if (this.stdin) |stdin| stdin else true, if (this.stdout) |*stdout| stdout.closed() else true, if (this.stderr) |*stderr| stderr.closed() else true });
                 return ret;
             }
 
@@ -4528,7 +4529,8 @@ pub const Interpreter = struct {
                 this.next();
                 return;
             }
-            unreachable;
+
+            @panic("Expected Cmd child to be Assigns or Expansion. This indicates a bug in Bun. Please file a GitHub issue. ");
         }
 
         fn initSubproc(this: *Cmd) void {
@@ -7121,7 +7123,8 @@ pub const Interpreter = struct {
                 while (!(this.state == .err or this.state == .done)) {
                     switch (this.state) {
                         .waiting_io => return,
-                        .idle, .done, .err => unreachable,
+                        .idle => @panic("Unexpected \"idle\" state in Pwd. This indicates a bug in Bun. Please file a GitHub issue."),
+                        .done, .err => unreachable,
                     }
                 }
 
@@ -9686,7 +9689,7 @@ pub const Interpreter = struct {
 
             pub fn next(this: *Exit) void {
                 switch (this.state) {
-                    .idle => unreachable,
+                    .idle => @panic("Unexpected \"idle\" state in Exit. This indicates a bug in Bun. Please file a GitHub issue."),
                     .waiting_io => {
                         return;
                     },

diff --git a/test/js/bun/shell/leak.test.ts b/test/js/bun/shell/leak.test.ts
@@ -6,6 +6,7 @@ import { tmpdir, devNull } from "os";
 import { join } from "path";
 import { createTestBuilder } from "./util";
 const TestBuilder = createTestBuilder(import.meta.path);
+type TestBuilder = InstanceType<typeof TestBuilder>;
 
 $.env(bunEnv);
 $.cwd(process.cwd());
@@ -51,7 +52,7 @@ const TESTS: [name: string, builder: () => TestBuilder, runs?: number][] = [
 ];
 
 describe("fd leak", () => {
-  function fdLeakTest(name: string, builder: () => TestBuilder, runs: number = 500, threshold: number = 5) {
+  function fdLeakTest(name: string, builder: () => TestBuilder, runs: number = 1000, threshold: number = 5) {
     test(`fdleak_${name}`, async () => {
       Bun.gc(true);
       const baseline = openSync(devNull, "r");

diff --git a/test/js/bun/shell/test_builder.ts b/test/js/bun/shell/test_builder.ts
@@ -17,29 +17,29 @@ export function createTestBuilder(path: string) {
   });
 
   class TestBuilder {
-    private promise: { type: "ok"; val: ShellPromise } | { type: "err"; val: Error };
-    private _testName: string | undefined = undefined;
+    promise: { type: "ok"; val: ShellPromise } | { type: "err"; val: Error };
+    _testName: string | undefined = undefined;
 
-    private expected_stdout: string | ((stdout: string, tempdir: string) => void) = "";
-    private expected_stderr: string | ((stderr: string, tempdir: string) => void) = "";
-    private expected_exit_code: number = 0;
-    private expected_error: ShellError | string | boolean | undefined = undefined;
-    private file_equals: { [filename: string]: string } = {};
-    private _doesNotExist: string[] = [];
-    private _timeout: number | undefined = undefined;
+    expected_stdout: string | ((stdout: string, tempdir: string) => void) = "";
+    expected_stderr: string | ((stderr: string, tempdir: string) => void) = "";
+    expected_exit_code: number = 0;
+    expected_error: ShellError | string | boolean | undefined = undefined;
+    file_equals: { [filename: string]: string } = {};
+    _doesNotExist: string[] = [];
+    _timeout: number | undefined = undefined;
 
-    private tempdir: string | undefined = undefined;
-    private _env: { [key: string]: string } | undefined = undefined;
+    tempdir: string | undefined = undefined;
+    _env: { [key: string]: string } | undefined = undefined;
 
-    private __todo: boolean | string = false;
+    __todo: boolean | string = false;
 
-    static UNEXPECTED_SUBSHELL_ERROR_OPEN =
+    UNEXPECTED_SUBSHELL_ERROR_OPEN =
       "Unexpected `(`, subshells are currently not supported right now. Escape the `(` or open a GitHub issue.";
 
-    static UNEXPECTED_SUBSHELL_ERROR_CLOSE =
+    UNEXPECTED_SUBSHELL_ERROR_CLOSE =
       "Unexpected `)`, subshells are currently not supported right now. Escape the `)` or open a GitHub issue.";
 
-    constructor(promise: TestBuilder["promise"]) {
+    public constructor(promise: TestBuilder["promise"]) {
       this.promise = promise;
     }
 
@@ -53,7 +53,7 @@ export function createTestBuilder(path: string) {
    * TestBuilder.command`echo hi!`.stdout('hi!\n').runAsTest('echo works')
    * ```
    */
-    static command(strings: TemplateStringsArray, ...expressions: any[]): TestBuilder {
+    public static command(strings: TemplateStringsArray, ...expressions: any[]): TestBuilder {
       try {
         if (process.env.BUN_DEBUG_SHELL_LOG_CMD === "1") console.info("[ShellTestBuilder] Cmd", strings.join(""));
         const promise = Bun.$(strings, ...expressions).nothrow();
@@ -65,7 +65,7 @@ export function createTestBuilder(path: string) {
       }
     }
 
-    directory(path: string): this {
+    public directory(path: string): this {
       const tempdir = this.getTempDir();
       fs.mkdirSync(join(tempdir, path), { recursive: true });
       return this;