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

fix(view): dont immediately exit on first workspace 404 #7508

Merged
merged 2 commits into from
May 16, 2024

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented May 10, 2024

Fixes: #5444

This PR will continue running through all workspaces for npm view even when a workspace encounters an E404 error. This usually happens when you run npm view -ws but have private workspaces. A future iteration could log a different message if an E404 is encountered on a private workspace, but for this PR I wanted to keep it generic since there are a number of reasons a request for a package manifest could 404.

npm view . version -ws

# BEFORE
❯ npm view . version -ws
npm error code E404
npm error 404 Not Found - GET https://registry.npmjs.org/@npmcli%2fdocs - Not found
npm error 404
npm error 404  '@npmcli/docs@*' is not in this registry.
npm error 404
npm error 404 Note that you can also install from a
npm error 404 tarball, folder, http url, or git url.

npm error A complete log of this run can be found in: /Users/lukekarrys/.npm/_logs/2024-05-15T22_04_30_007Z-debug-0.log

# AFTER
❯ npmlocal view . version -ws
@npmcli/docs:
npm error code E404
npm error 404 Not Found - GET https://registry.npmjs.org/@npmcli%2fdocs - Not found
npm error 404
npm error 404  '@npmcli/docs@*' is not in this registry.
npm error 404
npm error 404 Note that you can also install from a
npm error 404 tarball, folder, http url, or git url.
@npmcli/smoke-tests:
npm error code E404
npm error 404 Not Found - GET https://registry.npmjs.org/@npmcli%2fsmoke-tests - Not found
npm error 404
npm error 404  '@npmcli/smoke-tests@*' is not in this registry.
npm error 404
npm error 404 Note that you can also install from a
npm error 404 tarball, folder, http url, or git url.
@npmcli/mock-globals:
npm error code E404
npm error 404 Not Found - GET https://registry.npmjs.org/@npmcli%2fmock-globals - Not found
npm error 404
npm error 404  '@npmcli/mock-globals@*' is not in this registry.
npm error 404
npm error 404 Note that you can also install from a
npm error 404 tarball, folder, http url, or git url.
@npmcli/mock-registry:
npm error code E404
npm error 404 Not Found - GET https://registry.npmjs.org/@npmcli%2fmock-registry - Not found
npm error 404
npm error 404  '@npmcli/mock-registry@*' is not in this registry.
npm error 404
npm error 404 Note that you can also install from a
npm error 404 tarball, folder, http url, or git url.
@npmcli/arborist:
7.5.1
@npmcli/config:
8.3.1
libnpmaccess:
8.0.5
libnpmdiff:
6.1.1
libnpmexec:
8.1.0
libnpmfund:
5.0.9
libnpmhook:
10.0.4
libnpmorg:
6.0.5
libnpmpack:
7.0.1
libnpmpublish:
9.0.7
libnpmsearch:
7.0.4
libnpmteam:
6.0.4
libnpmversion:
6.0.1

npm view . version -ws --json

# BEFORE
❯ npm view . version -ws --json
npm error code E404
npm error 404 Not Found - GET https://registry.npmjs.org/@npmcli%2fdocs - Not found
npm error 404
npm error 404  '@npmcli/docs@*' is not in this registry.
npm error 404
npm error 404 Note that you can also install from a
npm error 404 tarball, folder, http url, or git url.
{
  "error": {
    "code": "E404",
    "summary": "Not Found - GET https://registry.npmjs.org/@npmcli%2fdocs - Not found",
    "detail": "\n '@npmcli/docs@*' is not in this registry.\n\nNote that you can also install from a\ntarball, folder, http url, or git url."
  }
}

npm error A complete log of this run can be found in: /Users/lukekarrys/.npm/_logs/2024-05-15T22_04_55_076Z-debug-0.log

# AFTER
❯ npmlocal view . version -ws --json
{
  "@npmcli/arborist": "7.5.1",
  "@npmcli/config": "8.3.1",
  "libnpmaccess": "8.0.5",
  "libnpmdiff": "6.1.1",
  "libnpmexec": "8.1.0",
  "libnpmfund": "5.0.9",
  "libnpmhook": "10.0.4",
  "libnpmorg": "6.0.5",
  "libnpmpack": "7.0.1",
  "libnpmpublish": "9.0.7",
  "libnpmsearch": "7.0.4",
  "libnpmteam": "6.0.4",
  "libnpmversion": "6.0.1",
  "error": {
    "@npmcli/docs": {
      "code": "E404",
      "summary": "Not Found - GET https://registry.npmjs.org/@npmcli%2fdocs - Not found",
      "detail": "'@npmcli/docs@*' is not in this registry.\n\nNote that you can also install from a\ntarball, folder, http url, or git url."
    },
    "@npmcli/smoke-tests": {
      "code": "E404",
      "summary": "Not Found - GET https://registry.npmjs.org/@npmcli%2fsmoke-tests - Not found",
      "detail": "'@npmcli/smoke-tests@*' is not in this registry.\n\nNote that you can also install from a\ntarball, folder, http url, or git url."
    },
    "@npmcli/mock-globals": {
      "code": "E404",
      "summary": "Not Found - GET https://registry.npmjs.org/@npmcli%2fmock-globals - Not found",
      "detail": "'@npmcli/mock-globals@*' is not in this registry.\n\nNote that you can also install from a\ntarball, folder, http url, or git url."
    },
    "@npmcli/mock-registry": {
      "code": "E404",
      "summary": "Not Found - GET https://registry.npmjs.org/@npmcli%2fmock-registry - Not found",
      "detail": "'@npmcli/mock-registry@*' is not in this registry.\n\nNote that you can also install from a\ntarball, folder, http url, or git url."
    }
  }
}

@lukekarrys lukekarrys requested a review from a team as a code owner May 10, 2024 23:28
@lukekarrys lukekarrys marked this pull request as draft May 10, 2024 23:28
@npm-cli-bot
Copy link
Collaborator

npm-cli-bot commented May 10, 2024

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps
no-clean
audit
npm@latest 35.077 ±0.00 10.645 ±0.12 11.737 ±0.05 1.539 ±0.00 1.532 ±0.01 1.273 ±0.01 8.185 ±0.02 1.260 ±0.01 0.137 ±0.00 0.164 ±0.00 14.629 ±0.12 2.158 ±0.06
#7508 33.914 ±0.23 10.606 ±0.03 11.669 ±0.04 1.549 ±0.03 1.535 ±0.00 1.266 ±0.01 8.199 ±0.01 1.281 ±0.02 0.138 ±0.00 0.164 ±0.00 14.628 ±0.11 2.156 ±0.01
app-medium clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps
no-clean
audit
npm@latest 26.882 ±0.74 8.138 ±0.09 9.203 ±0.02 1.530 ±0.01 1.497 ±0.02 1.436 ±0.00 5.877 ±0.02 1.316 ±0.00 0.141 ±0.00 0.174 ±0.00 9.942 ±0.01 2.037 ±0.09
#7508 27.213 ±0.55 8.268 ±0.04 9.160 ±0.04 1.558 ±0.01 1.571 ±0.01 1.484 ±0.01 5.946 ±0.02 1.355 ±0.02 0.141 ±0.00 0.172 ±0.00 9.898 ±0.14 1.978 ±0.05

@lukekarrys lukekarrys force-pushed the lk/view-json-errors-workspace branch 4 times, most recently from b3de0de to 160d6e9 Compare May 11, 2024 04:46
@lukekarrys lukekarrys force-pushed the lk/view-json-errors-workspace branch 19 times, most recently from 3b959de to b4b4f22 Compare May 13, 2024 02:57
@lukekarrys lukekarrys changed the base branch from latest to lk/errors-and-json May 13, 2024 02:59
@lukekarrys lukekarrys force-pushed the lk/view-json-errors-workspace branch from b4b4f22 to 1bc9bb4 Compare May 13, 2024 04:26
@lukekarrys lukekarrys force-pushed the lk/view-json-errors-workspace branch from 1bc9bb4 to 76fd359 Compare May 13, 2024 07:01
Base automatically changed from lk/errors-and-json to latest May 13, 2024 17:24
@lukekarrys lukekarrys force-pushed the lk/view-json-errors-workspace branch 2 times, most recently from 1ede923 to a419471 Compare May 14, 2024 18:07
@lukekarrys lukekarrys changed the base branch from latest to lk/publish-output-buffer May 14, 2024 18:07
Base automatically changed from lk/publish-output-buffer to latest May 15, 2024 19:44
@lukekarrys lukekarrys force-pushed the lk/view-json-errors-workspace branch from a419471 to 39f157f Compare May 15, 2024 22:02
@lukekarrys lukekarrys changed the title fix(view): output full json object with errors with workspaces fix(view): dont exit on workspace 404 May 15, 2024
@lukekarrys lukekarrys marked this pull request as ready for review May 15, 2024 22:09
@lukekarrys lukekarrys changed the title fix(view): dont exit on workspace 404 fix(view): dont immediately exit on first workspace 404 May 15, 2024
@lukekarrys
Copy link
Contributor Author

This also removes the last two istanbul ignore comments in display.js now that this behavior in the view command uses those code paths.

@lukekarrys lukekarrys merged commit ef4c975 into latest May 16, 2024
23 checks passed
@lukekarrys lukekarrys deleted the lk/view-json-errors-workspace branch May 16, 2024 02:37
@github-actions github-actions bot mentioned this pull request May 16, 2024
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

Successfully merging this pull request may close these issues.

[BUG] npm view --json command returns not only JSON if failed
3 participants