diff --git a/packages/utils/src/locking.js b/packages/utils/src/locking.js index d0ee5f0f07..c9b7fc15ba 100644 --- a/packages/utils/src/locking.js +++ b/packages/utils/src/locking.js @@ -35,16 +35,33 @@ export async function getLockPath(config) { export async function lock({ id, dir, root, options }) { const lockPath = await getLockPath({ id, dir, root }) - const locked = await properlock.check(lockPath) - if (locked) { - consola.fatal(`A lock with id '${id}' already exists on ${dir}`) + try { + const locked = await properlock.check(lockPath) + if (locked) { + consola.fatal(`A lock with id '${id}' already exists on ${dir}`) + } + } catch (e) { + consola.debug(`Check for an existing lock with id '${id}' on ${dir} failed`, e) } - options = getLockOptions(options) - const release = await properlock.lock(lockPath, options) + let lockWasCompromised = false + let release + + try { + options = getLockOptions(options) + + const onCompromised = options.onCompromised + options.onCompromised = (err) => { + onCompromised(err) + lockWasCompromised = true + } + + release = await properlock.lock(lockPath, options) + } catch (e) {} if (!release) { consola.warn(`Unable to get a lock with id '${id}' on ${dir} (but will continue)`) + return false } if (!lockPaths.size) { @@ -59,8 +76,27 @@ export async function lock({ id, dir, root, options }) { lockPaths.add(lockPath) return async function lockRelease() { - await release() - await fs.remove(lockPath) - lockPaths.delete(lockPath) + try { + await fs.remove(lockPath) + lockPaths.delete(lockPath) + + // release as last so the lockPath is still removed + // when it fails on a compromised lock + await release() + } catch (e) { + if (!lockWasCompromised || !e.message.includes('already released')) { + consola.debug(e) + return + } + + // proper-lockfile doesnt remove lockDir when lock is compromised + // removing it here could cause the 'other' process to throw an error + // as well, but in our case its much more likely the lock was + // compromised due to mtime update timeouts + const lockDir = `${lockPath}.lock` + if (await fs.exists(lockDir)) { + await fs.remove(lockDir) + } + } } } diff --git a/packages/utils/test/locking.test.js b/packages/utils/test/locking.test.js index 225c64157c..52acfd6ddb 100644 --- a/packages/utils/test/locking.test.js +++ b/packages/utils/test/locking.test.js @@ -61,7 +61,7 @@ describe('util: locking', () => { }) test('lock creates a lock and returns a release fn', async () => { - properlock.lock.mockImplementationOnce(() => true) + properlock.lock.mockReturnValue(true) const fn = await lock(lockConfig) @@ -75,7 +75,7 @@ describe('util: locking', () => { }) test('lock throws error when lock already exists', async () => { - properlock.check.mockImplementationOnce(() => true) + properlock.check.mockReturnValue(true) await lock(lockConfig) expect(properlock.check).toHaveBeenCalledTimes(1) @@ -84,7 +84,19 @@ describe('util: locking', () => { }) test('lock logs warning when it couldnt get a lock', async () => { - properlock.lock.mockImplementationOnce(() => false) + properlock.lock.mockReturnValue(false) + + const fn = await lock(lockConfig) + expect(fn).toBe(false) + expect(properlock.lock).toHaveBeenCalledTimes(1) + expect(consola.warn).toHaveBeenCalledTimes(1) + expect(consola.warn).toHaveBeenCalledWith(`Unable to get a lock with id '${lockConfig.id}' on ${lockConfig.dir} (but will continue)`) + }) + + test('lock logs warning when proper.lock threw error', async () => { + properlock.lock.mockImplementation(() => { + throw new Error('test error') + }) await lock(lockConfig) expect(properlock.lock).toHaveBeenCalledTimes(1) @@ -94,7 +106,7 @@ describe('util: locking', () => { test('lock returns a release method for unlocking both lockfile as lockPath', async () => { const release = jest.fn() - properlock.lock.mockImplementationOnce(() => release) + properlock.lock.mockImplementation(() => release) const fn = await lock(lockConfig) await fn() @@ -105,7 +117,7 @@ describe('util: locking', () => { test('lock release also cleansup onExit set', async () => { const release = jest.fn() - properlock.lock.mockImplementationOnce(() => release) + properlock.lock.mockImplementation(() => release) const fn = await lock(lockConfig) expect(lockPaths.size).toBe(1) @@ -114,8 +126,58 @@ describe('util: locking', () => { expect(lockPaths.size).toBe(0) }) + test('lock release only logs error when error thrown', async () => { + const release = jest.fn(() => { + throw new Error('test error') + }) + properlock.lock.mockImplementation(() => release) + + const fn = await lock(lockConfig) + await expect(fn()).resolves.not.toThrow() + + expect(consola.debug).toHaveBeenCalledTimes(1) + }) + + test('lock check only logs error when error thrown', async () => { + const testError = new Error('check error') + properlock.lock.mockImplementation(() => () => {}) + properlock.check.mockImplementation(() => { + throw testError + }) + + const fn = await lock(lockConfig) + expect(fn).toEqual(expect.any(Function)) + + expect(consola.debug).toHaveBeenCalledTimes(1) + expect(consola.debug).toHaveBeenCalledWith(`Check for an existing lock with id '${lockConfig.id}' on ${lockConfig.dir} failed`, testError) + }) + + test('lock release doesnt log error when error thrown because lock compromised', async () => { + fs.exists.mockReturnValue(true) + const testError = new Error('Lock is already released') + const release = jest.fn(() => { + throw testError + }) + + properlock.lock.mockImplementation((path, options) => { + options.onCompromised() + return release + }) + + const fn = await lock({ + ...lockConfig, + options: { + // overwrite default compromised which calls consola.warn + onCompromised() {} + } + }) + + await expect(fn()).resolves.not.toThrow() + expect(consola.warn).not.toHaveBeenCalled() + }) + test('lock sets exit listener once to remove lockPaths', async () => { - properlock.lock.mockImplementationOnce(() => true) + properlock.lock.mockReturnValue(true) await lock(lockConfig) await lock(lockConfig) @@ -124,8 +186,10 @@ describe('util: locking', () => { }) test('exit listener removes all lockPaths when called', async () => { + properlock.lock.mockReturnValue(true) + let callback - onExit.mockImplementationOnce(cb => (callback = cb)) + onExit.mockImplementation(cb => (callback = cb)) const lockConfig2 = Object.assign({}, lockConfig, { id: 'id2' }) @@ -145,7 +209,7 @@ describe('util: locking', () => { }) test('lock uses setLockOptions to set defaults', async () => { - const spy = properlock.lock.mockImplementationOnce(() => true) + const spy = properlock.lock.mockReturnValue(true) await lock(lockConfig)