Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apps or names cannnot try to open #272

Open
masterX89 opened this issue Jan 5, 2022 · 2 comments
Open

Apps or names cannnot try to open #272

masterX89 opened this issue Jan 5, 2022 · 2 comments

Comments

@masterX89
Copy link

Basic Info

  1. Windows

    • System: Windows 10 x64 (19043.1348)

    • Browser:

      • Not have Firefox
      • Microsoft Edge 96.0.1054.62
      • Chrome 96.0.4664.110
  2. MacOS

    • System: MacOS Monterey 12.0.1
    • Browser:
      • Not have Firefox
      • Chrome 97.0.4692.71
      • Safari 15.1 (17612.2.9.1.20)

What is expected?

app

Type:

{name: string | string[], arguments?: string[]} | Array<{name: string | string[], arguments: string[]}>

Specify the name of the app to open the target with, and optionally, app arguments. app can be an array of apps to try to open and name can be an array of app names to try. If each app fails, the last error will be thrown.

Steps to reproduce

I wrote the following code in the Windows which is consistent with the description in Basic Info. If the description of app api in the options is correct, then the web page will be opened by the chrome browser because there is no firefox browser. But the current situation is that the chrome browser or the msedge browser does not open the web page.

await open("https://google.com", {
    app: {
    	name: ["firefox", "chrome", "msedge"],
    },
});
await open("https://google.com", {
    app: {
        name: ["firefox", "msedge", "chrome"],
    },
});

Then I wrote the similar code that change in the MacOS which is consistent with the description in Basic Info, just replaced msedge with safari. The browser still does not open the web page.

await open("https://google.com", {
    app: {
    	name: ["firefox", "google chrome", "safari"],
    },
});
await open("https://google.com", {
    app: {
        name: ["firefox", "safari", "google chrome"],
    },
});

Analysis

After debugging, I think the problem lies in the pTryEach function:

const pTryEach = async (array, mapper) => {
	let latestError;

	for (const item of array) {
		try {
			return await mapper(item); // eslint-disable-line no-await-in-loop
		} catch (error) {
			latestError = error;
		}
	}

	throw latestError;
};

The mapper function calls the baseOpen function, and no exception is thrown in the baseOpen function, so the catch is actually an unreachable statement.

After reading the api of child_process in node.js, I think we should refer to options.stdio to modify childProcessOptions

const childProcessOptions = {
	stdio: ['pipe', 'pipe', process.stderr]
};

And add the following code after the statement to throw an exception:

subprocess.stderr.on('error', (error) => {
	throw new Error(`stderr: ${error}`);
});

Please confirm whether the bug exists, and I will submit a pr to fix it after confirmation.

@sindresorhus
Copy link
Owner

// @Richienb

@JohnCosta27
Copy link

This i still an issue for me, and the open pr #289 seems to solve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants