From d096588f0853208ae2cd6a08a1bfa571d02da6cf Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 25 Feb 2025 12:49:08 -0500 Subject: [PATCH] cache: wrap create failures in ReserveCacheError --- packages/cache/__tests__/saveCacheV2.test.ts | 39 ++++++++++++++++++-- packages/cache/src/cache.ts | 14 +++++-- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index ea3081ab..e96c2ac9 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -92,14 +92,14 @@ test('save with large cache outputs should fail using', async () => { expect(getCompressionMock).toHaveBeenCalledTimes(1) }) -test('create cache entry failure', async () => { +test('create cache entry failure on non-ok response', async () => { const paths = ['node_modules'] const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const infoLogMock = jest.spyOn(core, 'info') const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') - .mockReturnValue(Promise.resolve({ok: false, signedUploadUrl: ''})) + .mockResolvedValue({ok: false, signedUploadUrl: ''}) const createTarMock = jest.spyOn(tar, 'createTar') const finalizeCacheEntryMock = jest.spyOn( @@ -109,7 +109,7 @@ test('create cache entry failure', async () => { const compression = CompressionMethod.Zstd const getCompressionMock = jest .spyOn(cacheUtils, 'getCompressionMethod') - .mockReturnValueOnce(Promise.resolve(compression)) + .mockResolvedValueOnce(compression) const archiveFileSize = 1024 jest .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') @@ -133,6 +133,39 @@ test('create cache entry failure', async () => { expect(saveCacheMock).toHaveBeenCalledTimes(0) }) +test('create cache entry fails on rejected promise', async () => { + const paths = ['node_modules'] + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const infoLogMock = jest.spyOn(core, 'info') + + const createCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockRejectedValue(new Error('Failed to create cache entry')) + + const createTarMock = jest.spyOn(tar, 'createTar') + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockResolvedValueOnce(compression) + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const cacheId = await saveCache(paths, key) + expect(cacheId).toBe(-1) + expect(infoLogMock).toHaveBeenCalledWith( + `Failed to save: Unable to reserve cache with key ${key}, another job may be creating this cache.` + ) + + expect(createCacheEntryMock).toHaveBeenCalledWith({ + key, + version: cacheUtils.getCacheVersion(paths, compression) + }) + expect(createTarMock).toHaveBeenCalledTimes(1) + expect(getCompressionMock).toHaveBeenCalledTimes(1) +}) + test('save cache fails if a signedUploadURL was not passed', async () => { const paths = 'node_modules' const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 5f2102cb..9cbab6e0 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -525,8 +525,16 @@ async function saveCacheV2( version } - const response = await twirpClient.CreateCacheEntry(request) - if (!response.ok) { + let signedUploadUrl + + try { + const response = await twirpClient.CreateCacheEntry(request) + if (!response.ok) { + throw new Error('Response was not ok') + } + signedUploadUrl = response.signedUploadUrl + } catch (error) { + core.debug(`Failed to reserve cache: ${error}`) throw new ReserveCacheError( `Unable to reserve cache with key ${key}, another job may be creating this cache.` ) @@ -536,7 +544,7 @@ async function saveCacheV2( await cacheHttpClient.saveCache( cacheId, archivePath, - response.signedUploadUrl, + signedUploadUrl, options )