Skip to content

Commit 0a71ebb

Browse files
wraithgarlukekarrys
wraithgar
authored and
lukekarrys
committed
fix: stop retrying on 409 conflict
BREAKING CHANGE: libnpmpublish will no longer attempt a single automatic retry on 409 responses during publish.
1 parent d2c3712 commit 0a71ebb

File tree

2 files changed

+9
-341
lines changed

2 files changed

+9
-341
lines changed

‎workspaces/libnpmpublish/lib/publish.js

+9-80
Original file line numberDiff line numberDiff line change
@@ -50,42 +50,16 @@ Remove the 'private' field from the package.json to publish it.`),
5050
opts
5151
)
5252

53-
try {
54-
const res = await npmFetch(spec.escapedName, {
55-
...opts,
56-
method: 'PUT',
57-
body: metadata,
58-
ignoreBody: true,
59-
})
60-
if (transparencyLogUrl) {
61-
res.transparencyLogUrl = transparencyLogUrl
62-
}
63-
return res
64-
} catch (err) {
65-
if (err.code !== 'E409') {
66-
throw err
67-
}
68-
// if E409, we attempt exactly ONE retry, to protect us
69-
// against malicious activity like trying to publish
70-
// a bunch of new versions of a package at the same time
71-
// and/or spamming the registry
72-
const current = await npmFetch.json(spec.escapedName, {
73-
...opts,
74-
query: { write: true },
75-
})
76-
const newMetadata = patchMetadata(current, metadata)
77-
const res = await npmFetch(spec.escapedName, {
78-
...opts,
79-
method: 'PUT',
80-
body: newMetadata,
81-
ignoreBody: true,
82-
})
83-
/* istanbul ignore next */
84-
if (transparencyLogUrl) {
85-
res.transparencyLogUrl = transparencyLogUrl
86-
}
87-
return res
53+
const res = await npmFetch(spec.escapedName, {
54+
...opts,
55+
method: 'PUT',
56+
body: metadata,
57+
ignoreBody: true,
58+
})
59+
if (transparencyLogUrl) {
60+
res.transparencyLogUrl = transparencyLogUrl
8861
}
62+
return res
8963
}
9064

9165
const patchManifest = (_manifest, opts) => {
@@ -195,51 +169,6 @@ const buildMetadata = async (registry, manifest, tarballData, spec, opts) => {
195169
}
196170
}
197171

198-
const patchMetadata = (current, newData) => {
199-
const curVers = Object.keys(current.versions || {})
200-
.map(v => semver.clean(v, true))
201-
.concat(Object.keys(current.time || {})
202-
.map(v => semver.valid(v, true) && semver.clean(v, true))
203-
.filter(v => v))
204-
205-
const newVersion = Object.keys(newData.versions)[0]
206-
207-
if (curVers.indexOf(newVersion) !== -1) {
208-
const { name: pkgid, version } = newData
209-
throw Object.assign(
210-
new Error(
211-
`Cannot publish ${pkgid}@${version} over existing version.`
212-
), {
213-
code: 'EPUBLISHCONFLICT',
214-
pkgid,
215-
version,
216-
})
217-
}
218-
219-
current.versions = current.versions || {}
220-
current.versions[newVersion] = newData.versions[newVersion]
221-
for (const i in newData) {
222-
switch (i) {
223-
// objects that copy over the new stuffs
224-
case 'dist-tags':
225-
case 'versions':
226-
case '_attachments':
227-
for (const j in newData[i]) {
228-
current[i] = current[i] || {}
229-
current[i][j] = newData[i][j]
230-
}
231-
break
232-
233-
// copy
234-
default:
235-
current[i] = newData[i]
236-
break
237-
}
238-
}
239-
240-
return current
241-
}
242-
243172
// Check that all the prereqs are met for provenance generation
244173
const ensureProvenanceGeneration = async (registry, spec, opts) => {
245174
if (ciInfo.GITHUB_ACTIONS) {

‎workspaces/libnpmpublish/test/publish.js

-261
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict'
22

3-
const cloneDeep = require('lodash.clonedeep')
43
const crypto = require('crypto')
54
const fs = require('fs')
65
const npa = require('npm-package-arg')
@@ -180,266 +179,6 @@ t.test('scoped publish - restricted access', async t => {
180179
t.ok(ret, 'publish succeeded')
181180
})
182181

183-
t.test('retry after a conflict', async t => {
184-
const { publish } = t.mock('..')
185-
const registry = new MockRegistry({
186-
tap: t,
187-
registry: opts.registry,
188-
authorization: token,
189-
})
190-
const REV = '72-47f2986bfd8e8b55068b204588bbf484'
191-
const manifest = {
192-
name: 'libnpmpublish',
193-
version: '1.0.0',
194-
description: 'some stuff',
195-
}
196-
197-
const basePackument = {
198-
name: 'libnpmpublish',
199-
description: 'some stuff',
200-
access: 'public',
201-
_id: 'libnpmpublish',
202-
'dist-tags': {},
203-
versions: {},
204-
_attachments: {},
205-
}
206-
const currentPackument = cloneDeep({
207-
...basePackument,
208-
time: {
209-
modified: new Date().toISOString(),
210-
created: new Date().toISOString(),
211-
'1.0.1': new Date().toISOString(),
212-
},
213-
'dist-tags': { latest: '1.0.1' },
214-
versions: {
215-
'1.0.1': {
216-
_id: '[email protected]',
217-
_nodeVersion: process.versions.node,
218-
_npmVersion: opts.npmVersion,
219-
name: 'libnpmpublish',
220-
version: '1.0.1',
221-
description: 'some stuff',
222-
dist: {
223-
shasum,
224-
integrity: integrity.toString(),
225-
tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.1.tgz',
226-
},
227-
},
228-
},
229-
_attachments: {
230-
'libnpmpublish-1.0.1.tgz': {
231-
content_type: 'application/octet-stream',
232-
data: tarData.toString('base64'),
233-
length: tarData.length,
234-
},
235-
},
236-
})
237-
const newPackument = cloneDeep({
238-
...basePackument,
239-
'dist-tags': { latest: '1.0.0' },
240-
versions: {
241-
'1.0.0': {
242-
_id: '[email protected]',
243-
_nodeVersion: process.versions.node,
244-
_npmVersion: opts.npmVersion,
245-
name: 'libnpmpublish',
246-
version: '1.0.0',
247-
description: 'some stuff',
248-
dist: {
249-
shasum,
250-
integrity: integrity.toString(),
251-
tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.0.tgz',
252-
},
253-
},
254-
},
255-
_attachments: {
256-
'libnpmpublish-1.0.0.tgz': {
257-
content_type: 'application/octet-stream',
258-
data: tarData.toString('base64'),
259-
length: tarData.length,
260-
},
261-
},
262-
})
263-
const mergedPackument = cloneDeep({
264-
...basePackument,
265-
time: currentPackument.time,
266-
'dist-tags': { latest: '1.0.0' },
267-
versions: { ...currentPackument.versions, ...newPackument.versions },
268-
_attachments: { ...currentPackument._attachments, ...newPackument._attachments },
269-
})
270-
271-
registry.nock.put('/libnpmpublish', body => {
272-
t.notOk(body._rev, 'no _rev in initial post')
273-
t.same(body, newPackument, 'got conflicting packument')
274-
return true
275-
}).reply(409, { error: 'gimme _rev plz' })
276-
277-
registry.nock.get('/libnpmpublish?write=true').reply(200, {
278-
_rev: REV,
279-
...currentPackument,
280-
})
281-
282-
registry.nock.put('/libnpmpublish', body => {
283-
t.same(body, {
284-
_rev: REV,
285-
...mergedPackument,
286-
}, 'posted packument includes _rev and a merged version')
287-
return true
288-
}).reply(201, {})
289-
290-
const ret = await publish(manifest, tarData, opts)
291-
t.ok(ret, 'publish succeeded')
292-
})
293-
294-
t.test('retry after a conflict -- no versions on remote', async t => {
295-
const { publish } = t.mock('..')
296-
const registry = new MockRegistry({
297-
tap: t,
298-
registry: opts.registry,
299-
authorization: token,
300-
})
301-
const REV = '72-47f2986bfd8e8b55068b204588bbf484'
302-
const manifest = {
303-
name: 'libnpmpublish',
304-
version: '1.0.0',
305-
description: 'some stuff',
306-
}
307-
308-
const basePackument = {
309-
name: 'libnpmpublish',
310-
description: 'some stuff',
311-
access: 'public',
312-
_id: 'libnpmpublish',
313-
}
314-
const currentPackument = { ...basePackument }
315-
const newPackument = cloneDeep({
316-
...basePackument,
317-
'dist-tags': { latest: '1.0.0' },
318-
versions: {
319-
'1.0.0': {
320-
_id: '[email protected]',
321-
_nodeVersion: process.versions.node,
322-
_npmVersion: opts.npmVersion,
323-
name: 'libnpmpublish',
324-
version: '1.0.0',
325-
description: 'some stuff',
326-
dist: {
327-
shasum,
328-
integrity: integrity.toString(),
329-
tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.0.tgz',
330-
},
331-
},
332-
},
333-
_attachments: {
334-
'libnpmpublish-1.0.0.tgz': {
335-
content_type: 'application/octet-stream',
336-
data: tarData.toString('base64'),
337-
length: tarData.length,
338-
},
339-
},
340-
})
341-
const mergedPackument = cloneDeep({
342-
...basePackument,
343-
'dist-tags': { latest: '1.0.0' },
344-
versions: { ...newPackument.versions },
345-
_attachments: { ...newPackument._attachments },
346-
})
347-
348-
registry.nock.put('/libnpmpublish', body => {
349-
t.notOk(body._rev, 'no _rev in initial post')
350-
t.same(body, newPackument, 'got conflicting packument')
351-
return true
352-
}).reply(409, { error: 'gimme _rev plz' })
353-
354-
registry.nock.get('/libnpmpublish?write=true').reply(200, {
355-
_rev: REV,
356-
...currentPackument,
357-
})
358-
359-
registry.nock.put('/libnpmpublish', body => {
360-
t.same(body, {
361-
_rev: REV,
362-
...mergedPackument,
363-
}, 'posted packument includes _rev and a merged version')
364-
return true
365-
}).reply(201, {})
366-
367-
const ret = await publish(manifest, tarData, opts)
368-
369-
t.ok(ret, 'publish succeeded')
370-
})
371-
372-
t.test('version conflict', async t => {
373-
const { publish } = t.mock('..')
374-
const registry = new MockRegistry({
375-
tap: t,
376-
registry: opts.registry,
377-
authorization: token,
378-
})
379-
const REV = '72-47f2986bfd8e8b55068b204588bbf484'
380-
const manifest = {
381-
name: 'libnpmpublish',
382-
version: '1.0.0',
383-
description: 'some stuff',
384-
}
385-
386-
const basePackument = {
387-
name: 'libnpmpublish',
388-
description: 'some stuff',
389-
access: 'public',
390-
_id: 'libnpmpublish',
391-
'dist-tags': {},
392-
versions: {},
393-
_attachments: {},
394-
}
395-
const newPackument = cloneDeep(Object.assign({}, basePackument, {
396-
'dist-tags': { latest: '1.0.0' },
397-
versions: {
398-
'1.0.0': {
399-
_id: '[email protected]',
400-
_nodeVersion: process.versions.node,
401-
_npmVersion: '6.13.7',
402-
name: 'libnpmpublish',
403-
version: '1.0.0',
404-
description: 'some stuff',
405-
dist: {
406-
shasum,
407-
integrity: integrity.toString(),
408-
tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.0.tgz',
409-
},
410-
},
411-
},
412-
_attachments: {
413-
'libnpmpublish-1.0.0.tgz': {
414-
content_type: 'application/octet-stream',
415-
data: tarData.toString('base64'),
416-
length: tarData.length,
417-
},
418-
},
419-
}))
420-
421-
registry.nock.put('/libnpmpublish', body => {
422-
t.notOk(body._rev, 'no _rev in initial post')
423-
t.same(body, newPackument, 'got conflicting packument')
424-
return true
425-
}).reply(409, { error: 'gimme _rev plz' })
426-
427-
registry.nock.get('/libnpmpublish?write=true').reply(200, {
428-
_rev: REV,
429-
...newPackument,
430-
})
431-
432-
try {
433-
await publish(manifest, tarData, {
434-
...opts,
435-
npmVersion: '6.13.7',
436-
token: 'deadbeef',
437-
})
438-
} catch (err) {
439-
t.equal(err.code, 'EPUBLISHCONFLICT', 'got publish conflict code')
440-
}
441-
})
442-
443182
t.test('refuse if package marked private', async t => {
444183
const { publish } = t.mock('..')
445184
const registry = new MockRegistry({

0 commit comments

Comments
 (0)