Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,18 @@ const baseOpen = async options => {
// When we're in a fallback attempt, we need to detect launch failures before trying the next app.
// Wait for the close event to check the exit code before unreffing.
// The launcher (open/xdg-open/PowerShell) exits quickly (~10-30ms) even on success.
if (isFallbackAttempt) {
//
// On Windows, the actual command always runs inside a `powershell.exe` launcher (see
// above). Unlike `open`/`xdg-open`, this launcher can take noticeably longer than
// ~10-30ms to start executing the encoded script, so waiting only for `'spawn'` isn't
// enough: the promise can resolve, and the caller's process can exit, before
// `Start-Process` has actually run. Since the launcher isn't detached on Windows,
// exiting at that point can tear it down before it launches the target app. Always
// wait for the launcher's own `'close'` event on Windows to avoid this race. This does
// not wait for the *opened app* to close, only for the (fast) launcher script.
const isWindowsLauncher = platform === 'win32' || shouldUseWindowsInWsl;

if (isFallbackAttempt || isWindowsLauncher) {
return new Promise((resolve, reject) => {
subprocess.once('error', reject);

Expand Down
48 changes: 36 additions & 12 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,9 @@ test('subprocess is spawned before promise resolves', async t => {
t.true(childProcess.pid !== undefined && childProcess.pid !== null);
});

test.serial('app launches resolve before close without fallback', async t => {
const stubSpawnEmittingCloseAfter = delay => {
const originalSpawn = childProcess.spawn;
t.teardown(() => {
childProcess.spawn = originalSpawn;
});

let closeEmitted = false;
const state = {closeEmitted: false};

childProcess.spawn = () => {
// eslint-disable-next-line unicorn/prefer-event-target
Expand All @@ -155,18 +151,46 @@ test.serial('app launches resolve before close without fallback', async t => {
setImmediate(() => {
fakeChild.emit('spawn');
setTimeout(() => {
closeEmitted = true;
state.closeEmitted = true;
fakeChild.emit('close', 0);
}, 50);
}, delay);
});

return fakeChild;
};

const subprocess = await open('index.js', {app: {name: 'stub-app'}});
t.false(closeEmitted);
t.truthy(subprocess);
});
return {originalSpawn, state};
};

if (process.platform === 'win32') {
// On Windows the spawned process is always a `powershell.exe` launcher, which can take
// noticeably longer to start executing the encoded command than the ~10-30ms other
// platforms' launchers take. If the promise resolved before the launcher closes (as on
// other platforms), callers that exit right after `await open(...)` could tear down the
// non-detached launcher before it has actually started the target app.
// See https://github.com/sindresorhus/open/issues/298
test.serial('app launches on Windows resolve only after the launcher closes', async t => {
const {originalSpawn, state} = stubSpawnEmittingCloseAfter(50);
t.teardown(() => {
childProcess.spawn = originalSpawn;
});

const subprocess = await open('index.js', {app: {name: 'stub-app'}});
t.true(state.closeEmitted);
t.truthy(subprocess);
});
} else {
test.serial('app launches resolve before close without fallback', async t => {
const {originalSpawn, state} = stubSpawnEmittingCloseAfter(50);
t.teardown(() => {
childProcess.spawn = originalSpawn;
});

const subprocess = await open('index.js', {app: {name: 'stub-app'}});
t.false(state.closeEmitted);
t.truthy(subprocess);
});
}

test('fallback to next app when first app does not exist', async t => {
// Try nonexistent apps first, then a real app
Expand Down