diff --git a/packages/cache/__tests__/saveCache.test.ts b/packages/cache/__tests__/saveCache.test.ts index 5a4f8f9a..3beb33ca 100644 --- a/packages/cache/__tests__/saveCache.test.ts +++ b/packages/cache/__tests__/saveCache.test.ts @@ -237,7 +237,8 @@ test('save with server error should fail', async () => { .mockReturnValue( Promise.resolve({ ok: true, - signedUploadUrl: 'https://blob-storage.local?signed=true' + signedUploadUrl: 'https://blob-storage.local?signed=true', + message: "" }) ) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 317a3a53..d5bbe375 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -66,7 +66,7 @@ test('create cache entry failure on non-ok response', async () => { const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') - .mockResolvedValue({ok: false, signedUploadUrl: ''}) + .mockResolvedValue({ok: false, signedUploadUrl: '', message: ''}) const createTarMock = jest.spyOn(tar, 'createTar') const finalizeCacheEntryMock = jest.spyOn( @@ -149,7 +149,7 @@ test('save cache fails if a signedUploadURL was not passed', async () => { const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL, message: ''}) ) const createTarMock = jest.spyOn(tar, 'createTar') @@ -207,7 +207,7 @@ test('finalize save cache failure', async () => { const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL, message: ''}) ) const createTarMock = jest.spyOn(tar, 'createTar') @@ -227,7 +227,7 @@ test('finalize save cache failure', async () => { const finalizeCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') - .mockReturnValue(Promise.resolve({ok: false, entryId: ''})) + .mockReturnValue(Promise.resolve({ok: false, entryId: '', message: ''})) const cacheId = await saveCache([paths], key, options) @@ -286,7 +286,7 @@ test('save with valid inputs uploads a cache', async () => { jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL, message: ''}) ) const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') @@ -299,7 +299,7 @@ test('save with valid inputs uploads a cache', async () => { const finalizeCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') - .mockReturnValue(Promise.resolve({ok: true, entryId: cacheId.toString()})) + .mockReturnValue(Promise.resolve({ok: true, entryId: cacheId.toString(), message: ''})) const expectedCacheId = await saveCache([paths], key) @@ -327,6 +327,248 @@ test('save with valid inputs uploads a cache', async () => { expect(expectedCacheId).toBe(cacheId) }) +test('save with extremely large cache should succeed in v2 (no size limit)', async () => { + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(paths)] + const signedUploadURL = 'https://blob-storage.local?signed=true' + const createTarMock = jest.spyOn(tar, 'createTar') + + // Simulate a very large cache (20GB) + const archiveFileSize = 20 * 1024 * 1024 * 1024 // 20GB + const options: UploadOptions = { + archiveSizeBytes: archiveFileSize, + useAzureSdk: true, + uploadChunkSize: 64 * 1024 * 1024, + uploadConcurrency: 8 + } + + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const cacheId = 4 + jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL, message: ''}) + ) + + const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') + + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValue(Promise.resolve(compression)) + const cacheVersion = cacheUtils.getCacheVersion([paths], compression) + + const finalizeCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') + .mockReturnValue(Promise.resolve({ok: true, entryId: cacheId.toString(), message: ''})) + + const expectedCacheId = await saveCache([paths], key) + + const archiveFolder = '/foo/bar' + const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) + expect(saveCacheMock).toHaveBeenCalledWith( + -1, + archiveFile, + signedUploadURL, + options + ) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression + ) + + expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ + key, + version: cacheVersion, + sizeBytes: archiveFileSize.toString() + }) + + expect(getCompressionMock).toHaveBeenCalledTimes(1) + expect(expectedCacheId).toBe(cacheId) +}) + +test('save with create cache entry failure and specific error message', async () => { + const paths = ['node_modules'] + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const infoLogMock = jest.spyOn(core, 'info') + const warningLogMock = jest.spyOn(core, 'warning') + const errorMessage = 'Cache storage quota exceeded for repository' + + const createCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockResolvedValue({ok: false, signedUploadUrl: '', message: errorMessage}) + + 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(warningLogMock).toHaveBeenCalledWith( + `Cache reservation failed: ${errorMessage}` + ) + 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 with finalize cache entry failure and specific error message', async () => { + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(paths)] + const logWarningMock = jest.spyOn(core, 'warning') + const signedUploadURL = 'https://blob-storage.local?signed=true' + const archiveFileSize = 1024 + const errorMessage = 'Cache entry finalization failed due to concurrent access' + const options: UploadOptions = { + archiveSizeBytes: archiveFileSize, + useAzureSdk: true, + uploadChunkSize: 64 * 1024 * 1024, + uploadConcurrency: 8 + } + + const createCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL, message: ''}) + ) + + const createTarMock = jest.spyOn(tar, 'createTar') + const saveCacheMock = jest + .spyOn(cacheHttpClient, 'saveCache') + .mockResolvedValue(Promise.resolve()) + + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValueOnce(Promise.resolve(compression)) + + const cacheVersion = cacheUtils.getCacheVersion([paths], compression) + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const finalizeCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') + .mockReturnValue(Promise.resolve({ok: false, entryId: '', message: errorMessage})) + + const cacheId = await saveCache([paths], key, options) + + expect(createCacheEntryMock).toHaveBeenCalledWith({ + key, + version: cacheVersion + }) + + const archiveFolder = '/foo/bar' + const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression + ) + + expect(saveCacheMock).toHaveBeenCalledWith( + -1, + archiveFile, + signedUploadURL, + options + ) + expect(getCompressionMock).toHaveBeenCalledTimes(1) + + expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ + key, + version: cacheVersion, + sizeBytes: archiveFileSize.toString() + }) + + expect(cacheId).toBe(-1) + expect(logWarningMock).toHaveBeenCalledWith(errorMessage) +}) + +test('save with multiple large caches should succeed in v2 (testing 50GB)', async () => { + const paths = ['large-dataset', 'node_modules', 'build-artifacts'] + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = paths.map(p => path.resolve(p)) + const signedUploadURL = 'https://blob-storage.local?signed=true' + const createTarMock = jest.spyOn(tar, 'createTar') + + // Simulate an extremely large cache (50GB) + const archiveFileSize = 50 * 1024 * 1024 * 1024 // 50GB + const options: UploadOptions = { + archiveSizeBytes: archiveFileSize, + useAzureSdk: true, + uploadChunkSize: 64 * 1024 * 1024, + uploadConcurrency: 8 + } + + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const cacheId = 7 + jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL, message: ''}) + ) + + const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') + + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValue(Promise.resolve(compression)) + const cacheVersion = cacheUtils.getCacheVersion(paths, compression) + + const finalizeCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') + .mockReturnValue(Promise.resolve({ok: true, entryId: cacheId.toString(), message: ''})) + + const expectedCacheId = await saveCache(paths, key) + + const archiveFolder = '/foo/bar' + const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) + expect(saveCacheMock).toHaveBeenCalledWith( + -1, + archiveFile, + signedUploadURL, + options + ) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression + ) + + expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ + key, + version: cacheVersion, + sizeBytes: archiveFileSize.toString() + }) + + expect(getCompressionMock).toHaveBeenCalledTimes(1) + expect(expectedCacheId).toBe(cacheId) +}) + test('save with non existing path should not save cache using v2 saveCache', async () => { const path = 'node_modules' const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index ec134a01..1cc910f0 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -29,6 +29,14 @@ export class ReserveCacheError extends Error { } } +export class FinalizeCacheError extends Error { + constructor(message: string) { + super(message) + this.name = 'FinalizeCacheError' + Object.setPrototypeOf(this, FinalizeCacheError.prototype) + } +} + function checkPaths(paths: string[]): void { if (!paths || paths.length === 0) { throw new ValidationError( @@ -568,7 +576,10 @@ async function saveCacheV2( try { const response = await twirpClient.CreateCacheEntry(request) if (!response.ok) { - throw new Error('Response was not ok') + if (response.message) { + core.warning(`Cache reservation failed: ${response.message}`) + } + throw new Error(response.message || 'Response was not ok') } signedUploadUrl = response.signedUploadUrl } catch (error) { @@ -597,6 +608,9 @@ async function saveCacheV2( core.debug(`FinalizeCacheEntryUploadResponse: ${finalizeResponse.ok}`) if (!finalizeResponse.ok) { + if (finalizeResponse.message) { + throw new FinalizeCacheError(finalizeResponse.message) + } throw new Error( `Unable to finalize cache with key ${key}, another job may be finalizing this cache.` ) @@ -609,6 +623,8 @@ async function saveCacheV2( throw error } else if (typedError.name === ReserveCacheError.name) { core.info(`Failed to save: ${typedError.message}`) + } else if (typedError.name === FinalizeCacheError.name) { + core.warning(typedError.message) } else { // Log server errors (5xx) as errors, all other errors as warnings if ( diff --git a/packages/cache/src/generated/results/api/v1/cache.ts b/packages/cache/src/generated/results/api/v1/cache.ts index 5e998c37..309f7eec 100644 --- a/packages/cache/src/generated/results/api/v1/cache.ts +++ b/packages/cache/src/generated/results/api/v1/cache.ts @@ -50,6 +50,12 @@ export interface CreateCacheEntryResponse { * @generated from protobuf field: string signed_upload_url = 2; */ signedUploadUrl: string; + /** + * When !ok, this field may contain a human-readable error message used to create an annotation + * + * @generated from protobuf field: string message = 3; + */ + message: string; } /** * @generated from protobuf message github.actions.results.api.v1.FinalizeCacheEntryUploadRequest @@ -94,6 +100,12 @@ export interface FinalizeCacheEntryUploadResponse { * @generated from protobuf field: int64 entry_id = 2; */ entryId: string; + /** + * When !ok, this field may contain a human-readable error message used to create an annotation + * + * @generated from protobuf field: string message = 3; + */ + message: string; } /** * @generated from protobuf message github.actions.results.api.v1.GetCacheEntryDownloadURLRequest @@ -211,11 +223,12 @@ class CreateCacheEntryResponse$Type extends MessageType): CreateCacheEntryResponse { - const message = { ok: false, signedUploadUrl: "" }; + const message = { ok: false, signedUploadUrl: "", message: "" }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -232,6 +245,9 @@ class CreateCacheEntryResponse$Type extends MessageType): FinalizeCacheEntryUploadResponse { - const message = { ok: false, entryId: "0" }; + const message = { ok: false, entryId: "0", message: "" }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -354,6 +374,9 @@ class FinalizeCacheEntryUploadResponse$Type extends MessageType