Add a .success Promise getter to Subprocess.

[lgarron]

Implements #8313

What does this PR do?

Add a .success field to Subprocess that evaluates to a Promise which resolves/rejects based on the exit status of the process. This allows the following:

import { $ } from "bun";

await $`echo hi`;

… to be adapted into a spawn version as:

import { spawn } from "bun";

await spawn({cmd: ["echo", "hi"]}).success;

… instead of having to perform a check on the .exited value (which is easy to implement incorrectly, or to forget entirely):

import { default as assert } from "node:assert";
import { spawn } from "bun";

assert.equal(await spawn({cmd: ["echo", "hi"]}).exited, 0);

I was unable to find a place in JavaScript/TypeScript to modify the Subprocess object (e.g. under src/js/builtins), so I couldn't use the implementation suggested at #8313
I've tried my best to implement the same behaviour in Zig. However, the relevant code is event-based, so I couldn't implement the success Promise on top of the exited Promise and instead had to implement it independently using similar code.

• Documentation or TypeScript types (it's okay to leave the rest blank in this case)
• Code changes

How did you verify your code works?

• I included a test for the new code, or existing tests cover it
• I ran my tests locally and they pass (bun-debug test test-file-name.test)
• I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
• I included a test for the new code, or an existing test covers it
• JSValue used outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed
• I wrote TypeScript/JavaScript tests and they pass locally (bun-debug test test-file-name.test)
• I added TypeScript types for the new methods, getters, or setters

There are a lot of opportunities to replaced .exited with .success in tests, since those tests in fact actually rely on success of a subprocess. This provides a good demonstration of the value of .success, but I haven't updated tests that are not directly related to testing this PR implementation itself.

There is also an unfortunate caveat: await (subprocess).success cannot error in the current version of bun because it just evaluates to await undefined. This results in a false negative. Ideally, any project using the .success field should declare a minimum bun version upon using it for the first time, but this is not supported by bun itself yet: #5846

[lgarron]

Ideally each clause should return an exit code and there should be unified code to resolve/reject promise. I was unable to get this to work and could use syntax advice.

Suggested change

	switch (this.process.status) {
	const exitCode: ?u8 = switch (this.process.status) {

[lgarron]

I was unable to figure out how to construct an actual ShellError, since that's defined in TypeScript. I'm assuming I need to get it from globalThis, and could also use advice on this.

[lgarron]

I was unfortunately unable to run the tests in this file.

bun bd test 'test/js/bun/spawn/spawn.test.ts' fails with a Cannot find package 'detect-libc' error.

[alii]

Could you try run bun install in the test folder, and then cd back up to repo root and try again?

[lgarron]

That worked¹², thanks!

Would be nice if that was included in the documentation for compiling/testing.

Footnotes

1. Well, after 16 minutes of waiting for DuckDB to compile. I thought I was doing something wrong, but apparently that's a known issue? ↩

2. 5 tests seem to fail in this file even on main, but I'm gonna keep poking at it. ↩

[RiskyMH]

Some of the spawn tests seem to be failing in CI

https://buildkite.com/bun/bun/builds/15568#annotation-test/js/bun/spawn/spawn.test.ts

[lgarron]

The fundamental issue seems to be an "Unhandled error between tests" guard.

Even waiting for all the Promises to be settled before returning from the test doesn't seem to help, nor does explicitly catching the error from each one in turn: 786b690#diff-c35b2f1b943ffa65bbb4a7185146dcf64d13f6b2e732a14324a53943109c717dR589-R599

This one's a chin-scratcher.

[lgarron]

The signature for Process.exited did not make sense to me, so this is just my best guess. I don't know if I can figure this one out without help either.