diff --git a/test/lib/commands/unpublish.js b/test/lib/commands/unpublish.js index 4c8bc5e058afa..c1fde874e276d 100644 --- a/test/lib/commands/unpublish.js +++ b/test/lib/commands/unpublish.js @@ -228,6 +228,30 @@ t.test('unpublish --force no version set', async t => { t.equal(joinedOutput(), '- test-package') }) +t.test('unpublish surfaces a failed whole-package delete', async t => { + const { joinedOutput, npm } = await loadMockNpm(t, { + config: { + force: true, + ...auth, + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true }, times: 2 }) + registry.nock.delete(`/${pkg}/-rev/${manifest._rev}`).reply(404) + + await t.rejects( + npm.exec('unpublish', ['test-package']), + { code: 'E404' }, + 'should reject when the registry delete fails' + ) + t.equal(joinedOutput(), '') +}) + t.test('silent', async t => { const { joinedOutput, npm } = await loadMockNpm(t, { config: { diff --git a/workspaces/libnpmpublish/lib/unpublish.js b/workspaces/libnpmpublish/lib/unpublish.js index 983a5d2c39068..10a4dd22e8cfc 100644 --- a/workspaces/libnpmpublish/lib/unpublish.js +++ b/workspaces/libnpmpublish/lib/unpublish.js @@ -31,87 +31,88 @@ const unpublish = async (spec, opts) => { spec, } + const pkgUri = spec.escapedName + let pkg try { - const pkgUri = spec.escapedName - const pkg = await npmFetch.json(pkgUri, { + pkg = await npmFetch.json(pkgUri, { ...opts, query: { write: true }, }) + } catch (err) { + if (err.code !== 'E404') { + throw err + } - const version = spec.rawSpec - const allVersions = pkg.versions || {} - const versionData = allVersions[version] + return true + } - const rawSpecs = (!spec.rawSpec || spec.rawSpec === '*') - const onlyVersion = Object.keys(allVersions).length === 1 - const noVersions = !Object.keys(allVersions).length + const version = spec.rawSpec + const allVersions = pkg.versions || {} + const versionData = allVersions[version] - // if missing specific version, - // assumed unpublished - if (!versionData && !rawSpecs && !noVersions) { - return true - } + const rawSpecs = (!spec.rawSpec || spec.rawSpec === '*') + const onlyVersion = Object.keys(allVersions).length === 1 + const noVersions = !Object.keys(allVersions).length + + // if missing specific version, + // assumed unpublished + if (!versionData && !rawSpecs && !noVersions) { + return true + } + + // unpublish all versions of a package: + // - no specs supplied "npm unpublish foo" + // - all specs ("*") "npm unpublish foo@*" + // - there was only one version + // - has no versions field on packument + if (rawSpecs || onlyVersion || noVersions) { + await npmFetch(`${pkgUri}/-rev/${pkg._rev}`, { + ...opts, + method: 'DELETE', + ignoreBody: true, + }) + return true + } else { + const dist = allVersions[version].dist + delete allVersions[version] + + const latestVer = pkg['dist-tags'].latest - // unpublish all versions of a package: - // - no specs supplied "npm unpublish foo" - // - all specs ("*") "npm unpublish foo@*" - // - there was only one version - // - has no versions field on packument - if (rawSpecs || onlyVersion || noVersions) { - await npmFetch(`${pkgUri}/-rev/${pkg._rev}`, { - ...opts, - method: 'DELETE', - ignoreBody: true, - }) - return true - } else { - const dist = allVersions[version].dist - delete allVersions[version] - - const latestVer = pkg['dist-tags'].latest - - // deleting dist tags associated to version - Object.keys(pkg['dist-tags']).forEach(tag => { - if (pkg['dist-tags'][tag] === version) { - delete pkg['dist-tags'][tag] - } - }) - - if (latestVer === version) { - pkg['dist-tags'].latest = Object.keys( - allVersions - ).sort(semver.compareLoose).pop() + // deleting dist tags associated to version + Object.keys(pkg['dist-tags']).forEach(tag => { + if (pkg['dist-tags'][tag] === version) { + delete pkg['dist-tags'][tag] } + }) - delete pkg._revisions - delete pkg._attachments - - // Update packument with removed versions - await npmFetch(`${pkgUri}/-rev/${pkg._rev}`, { - ...opts, - method: 'PUT', - body: pkg, - ignoreBody: true, - }) - - // Remove the tarball itself - const { _rev } = await npmFetch.json(pkgUri, { - ...opts, - query: { write: true }, - }) - const tarballUrl = getPathname(dist.tarball, opts.registry) - await npmFetch(`${tarballUrl}/-rev/${_rev}`, { - ...opts, - method: 'DELETE', - ignoreBody: true, - }) - return true - } - } catch (err) { - if (err.code !== 'E404') { - throw err + if (latestVer === version) { + pkg['dist-tags'].latest = Object.keys( + allVersions + ).sort(semver.compareLoose).pop() } + delete pkg._revisions + delete pkg._attachments + + // Update packument with removed versions + await npmFetch(`${pkgUri}/-rev/${pkg._rev}`, { + ...opts, + method: 'PUT', + body: pkg, + ignoreBody: true, + }) + + // Remove the tarball itself + const { _rev } = await npmFetch.json(pkgUri, { + ...opts, + query: { write: true }, + }) + const tarballUrl = getPathname(dist.tarball, opts.registry) + await npmFetch(`${tarballUrl}/-rev/${_rev}`, { + ...opts, + method: 'DELETE', + ignoreBody: true, + }) return true } } diff --git a/workspaces/libnpmpublish/test/unpublish.js b/workspaces/libnpmpublish/test/unpublish.js index 6bb3c32e6d88f..9ae22a24eb3b4 100644 --- a/workspaces/libnpmpublish/test/unpublish.js +++ b/workspaces/libnpmpublish/test/unpublish.js @@ -107,6 +107,22 @@ t.test('404 considered a success', async t => { t.ok(ret, '@npmcli/npm-unpublish-test was unpublished') }) +t.test('404 from whole-package delete is not considered a success', async t => { + const registry = new MockRegistry({ + tap: t, + registry: opts.registry, + }) + const manifest = registry.manifest({ name: '@npmcli/npm-unpublish-test' }) + const spec = npa(manifest.name) + registry.package({ manifest, query: { write: true } }) + registry.nock.delete(`/${spec.escapedName}/-rev/${manifest._rev}`).reply(404) + await t.rejects( + unpublish('@npmcli/npm-unpublish-test', opts), + { code: 'E404' }, + 'surfaces a failed full-package delete' + ) +}) + t.test('non-404 errors', async t => { const registry = new MockRegistry({ tap: t,