Skip to content

Commit 0765289

Browse files
keegancsmithowlstronaut
authored andcommitted
fix: handle ENOTEMPTY errors in moveFile
Like EEXIST, we handle ENOTEMPTY when we fail to moveFile. A user reported they consistently could not update our CLI tool. [1] In that report it always had the same file it tried to rename to. I believe what is happening here is when NPM retires an old version it renames it to a deterministic path. However, I suspect a previous update was interrupted leaving a non-empty directory at the destination. When googling ENOTEMPTY all the results seem to be about npm failures with users suggesting clearing out node_modules. Maybe this will also solve those issues. [1]: https://gist.github.com/keegancsmith/5dac339583cc127031da6ed0e8512d47
1 parent 2030250 commit 0765289

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

workspaces/arborist/lib/arborist/reify.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ module.exports = cls => class Reifier extends cls {
510510
if (er.code === 'ENOENT') {
511511
return didMkdirp ? null : mkdir(dirname(to), { recursive: true }).then(() =>
512512
this[_renamePath](from, to, true))
513-
} else if (er.code === 'EEXIST') {
513+
} else if (er.code === 'EEXIST' || er.code === 'ENOTEMPTY') {
514514
return rm(to, { recursive: true, force: true }).then(() => moveFile(from, to))
515515
} else {
516516
throw er

workspaces/arborist/test/arborist/reify.js

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,72 @@ t.test('rollbacks', { buffered: false }, t => {
818818
return printTree(tree)
819819
})
820820

821+
t.test('fail retiring nodes because rm fails after enotempty', t => {
822+
const path = fixture(t, 'testing-bundledeps-3')
823+
createRegistry(t, true)
824+
const a = newArb({ path, installStrategy: 'nested' })
825+
const enotempty = new Error('rename fail')
826+
enotempty.code = 'ENOTEMPTY'
827+
const kRenamePath = Symbol.for('renamePath')
828+
const renamePath = a[kRenamePath]
829+
a[kRenamePath] = (from, to) => {
830+
a[kRenamePath] = renamePath
831+
failRename = enotempty
832+
failRm = true
833+
return a[kRenamePath](from, to)
834+
}
835+
const kRollback = Symbol.for('rollbackRetireShallowNodes')
836+
const rollbackRetireShallowNodes = a[kRollback]
837+
let rolledBack = false
838+
a[kRollback] = er => {
839+
rolledBack = true
840+
failRename = new Error('some other error')
841+
failRm = false
842+
t.match(er, new Error('rm fail'))
843+
a[kRollback] = rollbackRetireShallowNodes
844+
return a[kRollback](er).then(er => {
845+
failRename = null
846+
return er
847+
}, er => {
848+
failRename = null
849+
throw er
850+
})
851+
}
852+
853+
return t.rejects(a.reify({
854+
update: ['@isaacs/testing-bundledeps-parent'],
855+
}), new Error('rm fail'))
856+
.then(() => t.equal(rolledBack, true, 'rolled back'))
857+
})
858+
859+
t.test('fail retiring node with enotempty, but then rm fixes it', async t => {
860+
const path = fixture(t, 'testing-bundledeps-3')
861+
createRegistry(t, true)
862+
const a = newArb({ path, installStrategy: 'nested' })
863+
const enotempty = new Error('rename fail')
864+
enotempty.code = 'ENOTEMPTY'
865+
const kRenamePath = Symbol.for('renamePath')
866+
const renamePath = a[kRenamePath]
867+
a[kRenamePath] = (from, to) => {
868+
a[kRenamePath] = renamePath
869+
failRenameOnce = enotempty
870+
return a[kRenamePath](from, to)
871+
}
872+
const kRollback = Symbol.for('rollbackRetireShallowNodes')
873+
const rollbackRetireShallowNodes = a[kRollback]
874+
a[kRollback] = er => {
875+
t.fail('should not roll back!')
876+
a[kRollback] = rollbackRetireShallowNodes
877+
return a[kRollback](er)
878+
}
879+
880+
const tree = await a.reify({
881+
update: ['@isaacs/testing-bundledeps-parent'],
882+
save: false,
883+
})
884+
return printTree(tree)
885+
})
886+
821887
t.test('fail creating sparse tree', t => {
822888
t.teardown(() => failMkdir = null)
823889
const path = fixture(t, 'testing-bundledeps-3')

0 commit comments

Comments
 (0)