Skip to content

Commit 6ca609e

Browse files
authored
fix: ping and doctor commands fix for checking if registry is online (#7789)
Ping: Don't use cache so ping does not report ping sucess incorrectly if it's offline or no internet Doctor: Don't use cache for pinging the registry. Fixes: #5870 Fixes: #3576 Fixes: #4112 <details> <summary>Testing of ping and doctor </summary> ```sh # -- current npm last ping resuts in cached request replying PONG ~/workarea/npm-cli $ npm ping --registry=http://localhost:4873 -ddd npm verbose cli /Users/milaninfy/.nvm/versions/node/v22.9.0/bin/node /Users/milaninfy/.nvm/versions/node/v22.9.0/bin/npm npm info using [email protected] npm info using [email protected] npm silly config load:file:/Users/milaninfy/.nvm/versions/node/v22.9.0/lib/node_modules/npm/npmrc npm silly config load:file:/Users/milaninfy/workarea/npm-cli/.npmrc npm silly config load:file:/Users/milaninfy/.npmrc npm silly config load:file:/Users/milaninfy/.nvm/versions/node/v22.9.0/etc/npmrc npm verbose title npm ping npm verbose argv "ping" "--registry" "http://localhost:4873" "--loglevel" "silly" npm verbose logfile logs-max:10 dir:/Users/milaninfy/.npm/_logs/2024-09-26T20_37_04_583Z- npm verbose logfile /Users/milaninfy/.npm/_logs/2024-09-26T20_37_04_583Z-debug-0.log npm notice PING http://localhost:4873/ npm silly logfile start cleaning logs, removing 1 files npm silly logfile done cleaning log files npm http fetch GET http://localhost:4873/-/ping?write=true attempt 1 failed with ECONNREFUSED npm http fetch GET http://localhost:4873/-/ping?write=true attempt 2 failed with ECONNREFUSED npm http fetch GET http://localhost:4873/-/ping?write=true attempt 3 failed with ECONNREFUSED npm http fetch GET 200 http://localhost:4873/-/ping?write=true 70045ms (cache stale) npm notice PONG 70046ms npm verbose cwd /Users/milaninfy/workarea/npm-cli npm verbose os Darwin 23.6.0 npm verbose node v22.9.0 npm verbose npm v10.8.3 npm verbose exit 0 npm info ok # -- After the change npm last ping resuts in failure after retries ~/workarea/npm-cli $ lnpm ping --registry=http://localhost:4873 -ddd npm verbose cli /Users/milaninfy/.nvm/versions/node/v22.9.0/bin/node /Users/milaninfy/workarea/npm-cli/index.js npm info using [email protected] npm info using [email protected] npm silly config load:file:/Users/milaninfy/workarea/npm-cli/npmrc npm silly config load:file:/Users/milaninfy/workarea/npm-cli/.npmrc npm silly config load:file:/Users/milaninfy/.npmrc npm silly config load:file:/Users/milaninfy/.nvm/versions/node/v22.9.0/etc/npmrc npm verbose title npm ping npm verbose argv "ping" "--registry" "http://localhost:4873" "--loglevel" "silly" npm verbose logfile logs-max:10 dir:/Users/milaninfy/.npm/_logs/2024-09-26T20_38_51_059Z- npm verbose logfile /Users/milaninfy/.npm/_logs/2024-09-26T20_38_51_059Z-debug-0.log npm notice PING http://localhost:4873/ npm silly logfile start cleaning logs, removing 1 files npm http fetch GET http://localhost:4873/-/ping?write=true attempt 1 failed with ECONNREFUSED npm silly logfile done cleaning log files npm http fetch GET http://localhost:4873/-/ping?write=true attempt 2 failed with ECONNREFUSED npm http fetch GET http://localhost:4873/-/ping?write=true attempt 3 failed with ECONNREFUSED npm verbose type system npm verbose stack FetchError: request to http://localhost:4873/-/ping?write=true failed, reason: npm verbose stack at ClientRequest.<anonymous> (/Users/milaninfy/workarea/npm-cli/node_modules/minipass-fetch/lib/index.js:130:14) npm verbose stack at ClientRequest.emit (node:events:519:28) npm verbose stack at emitErrorEvent (node:_http_client:103:11) npm verbose stack at _destroy (node:_http_client:886:9) npm verbose stack at onSocketNT (node:_http_client:906:5) npm verbose stack at process.processTicksAndRejections (node:internal/process/task_queues:91:21) npm error code ECONNREFUSED npm error errno ECONNREFUSED npm error FetchError: request to http://localhost:4873/-/ping?write=true failed, reason: npm error at ClientRequest.<anonymous> (/Users/milaninfy/workarea/npm-cli/node_modules/minipass-fetch/lib/index.js:130:14) npm error at ClientRequest.emit (node:events:519:28) npm error at emitErrorEvent (node:_http_client:103:11) npm error at _destroy (node:_http_client:886:9) npm error at onSocketNT (node:_http_client:906:5) npm error at process.processTicksAndRejections (node:internal/process/task_queues:91:21) { npm error code: 'ECONNREFUSED', npm error errno: 'ECONNREFUSED', npm error type: 'system' npm error } npm error npm error If you are behind a proxy, please make sure that the npm error 'proxy' config is set properly. See: 'npm help config' npm verbose cwd /Users/milaninfy/workarea/npm-cli npm verbose os Darwin 23.6.0 npm verbose node v22.9.0 npm verbose npm v10.8.3 npm verbose exit 1 npm verbose code 1 npm error A complete log of this run can be found in: /Users/milaninfy/.npm/_logs/2024-09-26T20_38_51_059Z-debug-0.log # -- npm doctor ping resuts in success due to cache hit ~/workarea/npm-cli $ npm doctor --registry=http://localhost:4873 -ddd npm verbose cli /Users/milaninfy/.nvm/versions/node/v22.9.0/bin/node /Users/milaninfy/.nvm/versions/node/v22.9.0/bin/npm npm info using [email protected] npm info using [email protected] npm silly config load:file:/Users/milaninfy/.nvm/versions/node/v22.9.0/lib/node_modules/npm/npmrc npm silly config load:file:/Users/milaninfy/workarea/npm-cli/.npmrc npm silly config load:file:/Users/milaninfy/.npmrc npm silly config load:file:/Users/milaninfy/.nvm/versions/node/v22.9.0/etc/npmrc npm verbose title npm doctor npm verbose argv "doctor" "--registry" "http://localhost:4873" "--loglevel" "silly" npm verbose logfile logs-max:10 dir:/Users/milaninfy/.npm/_logs/2024-09-26T20_40_30_672Z- npm verbose logfile /Users/milaninfy/.npm/_logs/2024-09-26T20_40_30_672Z-debug-0.log npm info doctor Running checkup Connecting to the registry npm info doctor Pinging registry npm silly logfile start cleaning logs, removing 1 files npm silly logfile done cleaning log files npm http fetch GET http://localhost:4873/-/ping?write=true attempt 1 failed with ECONNREFUSED npm http fetch GET 200 http://localhost:4873/-/ping?write=true 48ms (cache stale) Ok # -- after the changes npm doctor ping correctly resuts in failure after retires ~/workarea/npm-cli $ lnpm doctor --registry=http://localhost:4873 -ddd npm verbose cli /Users/milaninfy/.nvm/versions/node/v22.9.0/bin/node /Users/milaninfy/workarea/npm-cli/index.js npm info using [email protected] npm info using [email protected] npm silly config load:file:/Users/milaninfy/workarea/npm-cli/npmrc npm silly config load:file:/Users/milaninfy/workarea/npm-cli/.npmrc npm silly config load:file:/Users/milaninfy/.npmrc npm silly config load:file:/Users/milaninfy/.nvm/versions/node/v22.9.0/etc/npmrc npm verbose title npm doctor npm verbose argv "doctor" "--registry" "http://localhost:4873" "--loglevel" "silly" npm verbose logfile logs-max:10 dir:/Users/milaninfy/.npm/_logs/2024-09-26T20_41_05_904Z- npm verbose logfile /Users/milaninfy/.npm/_logs/2024-09-26T20_41_05_904Z-debug-0.log npm info doctor Running checkup Connecting to the registry npm info doctor Pinging registry npm silly logfile start cleaning logs, removing 1 files npm http fetch GET http://localhost:4873/-/ping?write=true attempt 1 failed with ECONNREFUSED npm silly logfile done cleaning log files npm http fetch GET http://localhost:4873/-/ping?write=true attempt 2 failed with ECONNREFUSED npm http fetch GET http://localhost:4873/-/ping?write=true attempt 3 failed with ECONNREFUSED Not ok request to http://localhost:4873/-/ping?write=true failed, reason: ``` </detail>
1 parent 63d6a73 commit 6ca609e

File tree

5 files changed

+48
-30
lines changed

5 files changed

+48
-30
lines changed

lib/utils/ping.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
// used by the ping and doctor commands
33
const fetch = require('npm-registry-fetch')
44
module.exports = async (flatOptions) => {
5-
const res = await fetch('/-/ping?write=true', flatOptions)
5+
const res = await fetch('/-/ping', { ...flatOptions, cache: false })
66
return res.json().catch(() => ({}))
77
}

mock-registry/lib/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ class MockRegistry {
320320
}
321321

322322
ping ({ body = {}, responseCode = 200 } = {}) {
323-
this.nock = this.nock.get(this.fullPath('/-/ping?write=true')).reply(responseCode, body)
323+
this.nock = this.nock.get(this.fullPath('/-/ping')).reply(responseCode, body)
324324
}
325325

326326
// full unpublish of an entire package

tap-snapshots/test/lib/commands/doctor.js.test.cjs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,7 @@ Object {
11661166
exports[`test/lib/commands/doctor.js TAP ping 404 > ping 404 1`] = `
11671167
Connecting to the registry
11681168
Not ok
1169-
404 404 Not Found - GET https://registry.npmjs.org/-/ping?write=true
1169+
404 404 Not Found - GET https://registry.npmjs.org/-/ping
11701170
Checking npm version
11711171
Ok
11721172
current: v1.0.0, latest: v1.0.0
@@ -1226,7 +1226,7 @@ Object {
12261226
exports[`test/lib/commands/doctor.js TAP ping 404 in color > ping 404 in color 1`] = `
12271227
Connecting to the registry
12281228
Not ok
1229-
[36m404 404 Not Found - GET https://registry.npmjs.org/-/ping?write=true[39m
1229+
[36m404 404 Not Found - GET https://registry.npmjs.org/-/ping[39m
12301230
Checking npm version
12311231
Ok
12321232
current: v1.0.0, latest: v1.0.0
@@ -1286,7 +1286,7 @@ Object {
12861286
exports[`test/lib/commands/doctor.js TAP ping exception with code > ping failure 1`] = `
12871287
Connecting to the registry
12881288
Not ok
1289-
request to https://registry.npmjs.org/-/ping?write=true failed, reason: Test Error
1289+
request to https://registry.npmjs.org/-/ping failed, reason: Test Error
12901290
Checking npm version
12911291
Ok
12921292
current: v1.0.0, latest: v1.0.0
@@ -1346,7 +1346,7 @@ Object {
13461346
exports[`test/lib/commands/doctor.js TAP ping exception without code > ping failure 1`] = `
13471347
Connecting to the registry
13481348
Not ok
1349-
request to https://registry.npmjs.org/-/ping?write=true failed, reason: Test Error
1349+
request to https://registry.npmjs.org/-/ping failed, reason: Test Error
13501350
Checking npm version
13511351
Ok
13521352
current: v1.0.0, latest: v1.0.0

test/lib/commands/doctor.js

+24-24
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ t.test('all clear', async t => {
8989
...dirs,
9090
})
9191
tnock(t, npm.config.get('registry'))
92-
.get('/-/ping?write=true').reply(200, '{}')
92+
.get('/-/ping').reply(200, '{}')
9393
.get('/npm').reply(200, npmManifest(npm.version))
9494
tnock(t, 'https://nodejs.org')
9595
.get('/dist/index.json').reply(200, nodeVersions)
@@ -108,7 +108,7 @@ t.test('all clear in color', async t => {
108108
},
109109
})
110110
tnock(t, npm.config.get('registry'))
111-
.get('/-/ping?write=true').reply(200, '{}')
111+
.get('/-/ping').reply(200, '{}')
112112
.get('/npm').reply(200, npmManifest(npm.version))
113113
tnock(t, 'https://nodejs.org')
114114
.get('/dist/index.json').reply(200, nodeVersions)
@@ -127,7 +127,7 @@ t.test('silent success', async t => {
127127
...dirs,
128128
})
129129
tnock(t, npm.config.get('registry'))
130-
.get('/-/ping?write=true').reply(200, '{}')
130+
.get('/-/ping').reply(200, '{}')
131131
.get('/npm').reply(200, npmManifest(npm.version))
132132
tnock(t, 'https://nodejs.org')
133133
.get('/dist/index.json').reply(200, nodeVersions)
@@ -146,7 +146,7 @@ t.test('silent errors', async t => {
146146
...dirs,
147147
})
148148
tnock(t, npm.config.get('registry'))
149-
.get('/-/ping?write=true').reply(404, '{}')
149+
.get('/-/ping').reply(404, '{}')
150150
await t.rejects(npm.exec('doctor', ['ping']), {
151151
message: /Check logs/,
152152
})
@@ -161,7 +161,7 @@ t.test('ping 404', async t => {
161161
...dirs,
162162
})
163163
tnock(t, npm.config.get('registry'))
164-
.get('/-/ping?write=true').reply(404, '{}')
164+
.get('/-/ping').reply(404, '{}')
165165
.get('/npm').reply(200, npmManifest(npm.version))
166166
tnock(t, 'https://nodejs.org')
167167
.get('/dist/index.json').reply(200, nodeVersions)
@@ -182,7 +182,7 @@ t.test('ping 404 in color', async t => {
182182
},
183183
})
184184
tnock(t, npm.config.get('registry'))
185-
.get('/-/ping?write=true').reply(404, '{}')
185+
.get('/-/ping').reply(404, '{}')
186186
.get('/npm').reply(200, npmManifest(npm.version))
187187
tnock(t, 'https://nodejs.org')
188188
.get('/dist/index.json').reply(200, nodeVersions)
@@ -198,7 +198,7 @@ t.test('ping exception with code', async t => {
198198
...dirs,
199199
})
200200
tnock(t, npm.config.get('registry'))
201-
.get('/-/ping?write=true').replyWithError({ message: 'Test Error', code: 'TEST' })
201+
.get('/-/ping').replyWithError({ message: 'Test Error', code: 'TEST' })
202202
.get('/npm').reply(200, npmManifest(npm.version))
203203
tnock(t, 'https://nodejs.org')
204204
.get('/dist/index.json').reply(200, nodeVersions)
@@ -214,7 +214,7 @@ t.test('ping exception without code', async t => {
214214
...dirs,
215215
})
216216
tnock(t, npm.config.get('registry'))
217-
.get('/-/ping?write=true').replyWithError({ message: 'Test Error', code: false })
217+
.get('/-/ping').replyWithError({ message: 'Test Error', code: false })
218218
.get('/npm').reply(200, npmManifest(npm.version))
219219
tnock(t, 'https://nodejs.org')
220220
.get('/dist/index.json').reply(200, nodeVersions)
@@ -230,7 +230,7 @@ t.test('npm out of date', async t => {
230230
...dirs,
231231
})
232232
tnock(t, npm.config.get('registry'))
233-
.get('/-/ping?write=true').reply(200, '{}')
233+
.get('/-/ping').reply(200, '{}')
234234
.get('/npm').reply(200, npmManifest('2.0.0'))
235235
tnock(t, 'https://nodejs.org')
236236
.get('/dist/index.json').reply(200, nodeVersions)
@@ -255,7 +255,7 @@ t.test('node out of date - lts', async t => {
255255
...dirs,
256256
})
257257
tnock(t, npm.config.get('registry'))
258-
.get('/-/ping?write=true').reply(200, '{}')
258+
.get('/-/ping').reply(200, '{}')
259259
.get('/npm').reply(200, npmManifest(npm.version))
260260
tnock(t, 'https://nodejs.org')
261261
.get('/dist/index.json').reply(200, nodeVersions)
@@ -280,7 +280,7 @@ t.test('node out of date - current', async t => {
280280
...dirs,
281281
})
282282
tnock(t, npm.config.get('registry'))
283-
.get('/-/ping?write=true').reply(200, '{}')
283+
.get('/-/ping').reply(200, '{}')
284284
.get('/npm').reply(200, npmManifest(npm.version))
285285
tnock(t, 'https://nodejs.org')
286286
.get('/dist/index.json').reply(200, nodeVersions)
@@ -297,7 +297,7 @@ t.test('non-default registry', async t => {
297297
...dirs,
298298
})
299299
tnock(t, npm.config.get('registry'))
300-
.get('/-/ping?write=true').reply(200, '{}')
300+
.get('/-/ping').reply(200, '{}')
301301
.get('/npm').reply(200, npmManifest(npm.version))
302302
tnock(t, 'https://nodejs.org')
303303
.get('/dist/index.json').reply(200, nodeVersions)
@@ -318,7 +318,7 @@ t.test('missing git', async t => {
318318
...dirs,
319319
})
320320
tnock(t, npm.config.get('registry'))
321-
.get('/-/ping?write=true').reply(200, '{}')
321+
.get('/-/ping').reply(200, '{}')
322322
.get('/npm').reply(200, npmManifest(npm.version))
323323
tnock(t, 'https://nodejs.org')
324324
.get('/dist/index.json').reply(200, nodeVersions)
@@ -344,7 +344,7 @@ t.test('windows skips permissions checks', async t => {
344344
globalPrefixDir: {},
345345
})
346346
tnock(t, npm.config.get('registry'))
347-
.get('/-/ping?write=true').reply(200, '{}')
347+
.get('/-/ping').reply(200, '{}')
348348
.get('/npm').reply(200, npmManifest(npm.version))
349349
tnock(t, 'https://nodejs.org')
350350
.get('/dist/index.json').reply(200, nodeVersions)
@@ -361,7 +361,7 @@ t.test('missing global directories', async t => {
361361
globalPrefixDir: {},
362362
})
363363
tnock(t, npm.config.get('registry'))
364-
.get('/-/ping?write=true').reply(200, '{}')
364+
.get('/-/ping').reply(200, '{}')
365365
.get('/npm').reply(200, npmManifest(npm.version))
366366
tnock(t, 'https://nodejs.org')
367367
.get('/dist/index.json').reply(200, nodeVersions)
@@ -377,7 +377,7 @@ t.test('missing local node_modules', async t => {
377377
globalPrefixDir: dirs.globalPrefixDir,
378378
})
379379
tnock(t, npm.config.get('registry'))
380-
.get('/-/ping?write=true').reply(200, '{}')
380+
.get('/-/ping').reply(200, '{}')
381381
.get('/npm').reply(200, npmManifest(npm.version))
382382
tnock(t, 'https://nodejs.org')
383383
.get('/dist/index.json').reply(200, nodeVersions)
@@ -406,7 +406,7 @@ t.test('incorrect owner', async t => {
406406
...dirs,
407407
})
408408
tnock(t, npm.config.get('registry'))
409-
.get('/-/ping?write=true').reply(200, '{}')
409+
.get('/-/ping').reply(200, '{}')
410410
.get('/npm').reply(200, npmManifest(npm.version))
411411
tnock(t, 'https://nodejs.org')
412412
.get('/dist/index.json').reply(200, nodeVersions)
@@ -430,7 +430,7 @@ t.test('incorrect permissions', async t => {
430430
...dirs,
431431
})
432432
tnock(t, npm.config.get('registry'))
433-
.get('/-/ping?write=true').reply(200, '{}')
433+
.get('/-/ping').reply(200, '{}')
434434
.get('/npm').reply(200, npmManifest(npm.version))
435435
tnock(t, 'https://nodejs.org')
436436
.get('/dist/index.json').reply(200, nodeVersions)
@@ -458,7 +458,7 @@ t.test('error reading directory', async t => {
458458
...dirs,
459459
})
460460
tnock(t, npm.config.get('registry'))
461-
.get('/-/ping?write=true').reply(200, '{}')
461+
.get('/-/ping').reply(200, '{}')
462462
.get('/npm').reply(200, npmManifest(npm.version))
463463
tnock(t, 'https://nodejs.org')
464464
.get('/dist/index.json').reply(200, nodeVersions)
@@ -481,7 +481,7 @@ t.test('cacache badContent', async t => {
481481
...dirs,
482482
})
483483
tnock(t, npm.config.get('registry'))
484-
.get('/-/ping?write=true').reply(200, '{}')
484+
.get('/-/ping').reply(200, '{}')
485485
.get('/npm').reply(200, npmManifest(npm.version))
486486
tnock(t, 'https://nodejs.org')
487487
.get('/dist/index.json').reply(200, nodeVersions)
@@ -504,7 +504,7 @@ t.test('cacache reclaimedCount', async t => {
504504
...dirs,
505505
})
506506
tnock(t, npm.config.get('registry'))
507-
.get('/-/ping?write=true').reply(200, '{}')
507+
.get('/-/ping').reply(200, '{}')
508508
.get('/npm').reply(200, npmManifest(npm.version))
509509
tnock(t, 'https://nodejs.org')
510510
.get('/dist/index.json').reply(200, nodeVersions)
@@ -527,7 +527,7 @@ t.test('cacache missingContent', async t => {
527527
...dirs,
528528
})
529529
tnock(t, npm.config.get('registry'))
530-
.get('/-/ping?write=true').reply(200, '{}')
530+
.get('/-/ping').reply(200, '{}')
531531
.get('/npm').reply(200, npmManifest(npm.version))
532532
tnock(t, 'https://nodejs.org')
533533
.get('/dist/index.json').reply(200, nodeVersions)
@@ -558,7 +558,7 @@ t.test('discrete checks', t => {
558558
...dirs,
559559
})
560560
tnock(t, npm.config.get('registry'))
561-
.get('/-/ping?write=true').reply(200, '{}')
561+
.get('/-/ping').reply(200, '{}')
562562
await npm.exec('doctor', ['ping'])
563563
t.matchSnapshot(joinedOutput(), 'output')
564564
t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')
@@ -586,7 +586,7 @@ t.test('discrete checks', t => {
586586
...dirs,
587587
})
588588
tnock(t, npm.config.get('registry'))
589-
.get('/-/ping?write=true').reply(200, '{}')
589+
.get('/-/ping').reply(200, '{}')
590590
await npm.exec('doctor', ['registry'])
591591
t.matchSnapshot(joinedOutput(), 'output')
592592
t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs')

test/lib/commands/ping.js

+18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
const t = require('tap')
22
const { load: loadMockNpm } = require('../../fixtures/mock-npm.js')
33
const MockRegistry = require('@npmcli/mock-registry')
4+
const cacache = require('cacache')
5+
const path = require('node:path')
46

57
t.test('no details', async t => {
68
const { npm, logs, joinedOutput } = await loadMockNpm(t)
@@ -74,3 +76,19 @@ t.test('invalid json', async t => {
7476
details: {},
7577
})
7678
})
79+
t.test('fail when registry is unreachable even if request is cached', async t => {
80+
const { npm } = await loadMockNpm(t, {
81+
config: { registry: 'https://ur.npmlocal.npmtest/' },
82+
cacheDir: { _cacache: {} },
83+
})
84+
const url = `${npm.config.get('registry')}-/ping`
85+
const cache = path.join(npm.cache, '_cacache')
86+
await cacache.put(cache,
87+
`make-fetch-happen:request-cache:${url}`,
88+
'{}', { metadata: { url } }
89+
)
90+
t.rejects(npm.exec('ping', []), {
91+
code: 'ENOTFOUND',
92+
},
93+
'throws ENOTFOUND error')
94+
})

0 commit comments

Comments
 (0)