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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] nothrow option for async which #80

Closed
Benjin opened this issue Sep 15, 2021 · 2 comments · Fixed by #86
Closed

[FEATURE] nothrow option for async which #80

Benjin opened this issue Sep 15, 2021 · 2 comments · Fixed by #86
Labels
Enhancement new feature or improvement

Comments

@Benjin
Copy link

Benjin commented Sep 15, 2021

Somewhat self-explanatory, but this option exists for the which.sync and not which. It'd be nice to have it for both, and there doesn't seem to be anything inherently incompatible with the async implementation, just slightly messy.
@mcmire mcmire mentioned this issue Jul 27, 2022
Merged
@darcyclarke darcyclarke added the Enhancement new feature or improvement label Jul 28, 2022
@lukekarrys lukekarrys mentioned this issue Oct 30, 2022
Merged
@rlueder
Copy link

rlueder commented Nov 15, 2022 •
edited
Loading

It seems that nothrow: true is only working working for which.sync and still throws an error for the async version. To reproduce using which v.3.0.0

  try {
    which.sync(command, { nothrow: true })
  } catch (error) {
    console.error(`${error.message}`);
  }

has no console output as expected (ie. it's null) while

  try {
    which(command, { nothrow: true })
  } catch (error) {
    console.error(`${error.message}`);
  }

still throws:

/node_modules/zx/node_modules/which/which.js:10
  Object.assign(new Error(`not found: ${cmd}`), { code: 'ENOENT' })
                ^

Error: not found: xcodes
    at getNotFoundError (/node_modules/zx/node_modules/which/which.js:10:17)
    at /node_modules/zx/node_modules/which/which.js:57:18
    at new Promise (<anonymous>)
    at step (/node_modules/zx/node_modules/which/which.js:54:21)
    at /node_modules/zx/node_modules/which/which.js:71:22
    at new Promise (<anonymous>)
    at subStep (/node_modules/zx/node_modules/which/which.js:69:33)
    at /node_modules/zx/node_modules/which/which.js:80:22
    at /node_modules/isexe/index.js:42:5
    at /node_modules/isexe/mode.js:8:5 {
  code: 'ENOENT'
}

@lukekarrys
Copy link
Contributor

@rlueder it looks like you might have multiple versions of which in your tree because the example you pasted is referencing /node_modules/zx/node_modules/which/which.js:10. that file no longer exists and has been replaced by lib/index.js.

can you trying reinstalling which@3 and trying your example again?

@ivancuric ivancuric mentioned this issue Feb 23, 2023
Closed
@softwarenerd softwarenerd mentioned this issue Aug 10, 2024
Merged
softwarenerd added a commit to posit-dev/positron that referenced this issue Aug 10, 2024
@softwarenerd
Add missing dependencies entry for which, which was causing the w…
7ee0596 
…rong version of `which` to be used, which caused R not to load on some configurations of Windows (#4311)

<!--
  Describe briefly what problem this pull request resolves, or what
  new feature it introduces. Include screenshots of any new or altered
  UI. Link to any GitHub issues but avoid "magic" keywords that will 
  automatically close the issue. If there are any details about your 
approach that are unintuitive or you want to draw attention to, please
  describe them here.
-->

This PR addresses #4280.

This issue was tracked down to the fact that the `positron-r` extension
did not have an explicit `dependencies` entry for the `which` package,
but it did have a `devDependencies` entry for `@types/which`
(`"@types/which": "^3.0.0",`).

As a result of this, the `positron-r/node_modules` folder contained
`which` 2.0.2 (I believe) from:

```
mocha@^9.2.1:
  version "9.2.2"
  resolved "https://registry.npmjs.org/mocha/-/mocha-9.2.2.tgz"
  integrity sha512-L6XC3EdwT6YrIk0yXpavvLkn8h+EU+Y5UcCHKECyMbdUIxyMuZj4bX4U9e1nvnvUUvQVsV2VHQr5zLdcUkhW/g==
  dependencies:
    "@ungap/promise-all-settled" "1.1.2"
    ansi-colors "4.1.1"
    browser-stdout "1.3.1"
    chokidar "3.5.3"
    debug "4.3.3"
    diff "5.0.0"
    escape-string-regexp "4.0.0"
    find-up "5.0.0"
    glob "7.2.0"
    growl "1.10.5"
    he "1.2.0"
    js-yaml "4.1.0"
    log-symbols "4.1.0"
    minimatch "4.2.1"
    ms "2.1.3"
    nanoid "3.3.1"
    serialize-javascript "6.0.0"
    strip-json-comments "3.1.1"
    supports-color "8.1.1"
    which "2.0.2"
    workerpool "6.2.0"
    yargs "16.2.0"
    yargs-parser "20.2.4"
    yargs-unparser "2.0.0"
```

This version of `which` does not support the `nothrow` option for `async
which`, which we rely on in this code fragment from
`extensions/positron-r/src/provider.ts`:

```
	const whichR = await which('R', { nothrow: true }) as string;
```

Because of this, on some configurations of Windows, an error was being
thrown which caused R not to load.

For background on this, see:
npm/node-which#80. This was the exact same
problem.

### QA Notes

<!--
  Add additional information for QA on how to validate the change,
  paying special attention to the level of risk, adjacent areas that
  could be affected by the change, and any important contextual
  information not present in the linked issues.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement new feature or improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat!: add @npmcli/template-oss and modernize npm/node-which
4 participants
@rlueder @darcyclarke @lukekarrys @Benjin