new error state, tests to cover

This commit is contained in:
Ryan Ghadimi
2025-08-13 13:00:46 +00:00
parent 0fe20e9d56
commit 06f7fd9df1
4 changed files with 297 additions and 12 deletions
+2 -1
View File
@@ -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: ""
})
)
+248 -6
View File
@@ -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'
+17 -1
View File
@@ -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 (
+30 -4
View File
@@ -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
constructor() {
super("github.actions.results.api.v1.CreateCacheEntryResponse", [
{ no: 1, name: "ok", kind: "scalar", T: 8 /*ScalarType.BOOL*/ },
{ no: 2, name: "signed_upload_url", kind: "scalar", T: 9 /*ScalarType.STRING*/ }
{ no: 2, name: "signed_upload_url", kind: "scalar", T: 9 /*ScalarType.STRING*/ },
{ no: 3, name: "message", kind: "scalar", T: 9 /*ScalarType.STRING*/ }
]);
}
create(value?: PartialMessage<CreateCacheEntryResponse>): 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<CreateCacheEntryResponse>(this, message, value);
@@ -232,6 +245,9 @@ class CreateCacheEntryResponse$Type extends MessageType<CreateCacheEntryResponse
case /* string signed_upload_url */ 2:
message.signedUploadUrl = reader.string();
break;
case /* string message */ 3:
message.message = reader.string();
break;
default:
let u = options.readUnknownField;
if (u === "throw")
@@ -250,6 +266,9 @@ class CreateCacheEntryResponse$Type extends MessageType<CreateCacheEntryResponse
/* string signed_upload_url = 2; */
if (message.signedUploadUrl !== "")
writer.tag(2, WireType.LengthDelimited).string(message.signedUploadUrl);
/* string message = 3; */
if (message.message !== "")
writer.tag(3, WireType.LengthDelimited).string(message.message);
let u = options.writeUnknownFields;
if (u !== false)
(u == true ? UnknownFieldHandler.onWrite : u)(this.typeName, message, writer);
@@ -333,11 +352,12 @@ class FinalizeCacheEntryUploadResponse$Type extends MessageType<FinalizeCacheEnt
constructor() {
super("github.actions.results.api.v1.FinalizeCacheEntryUploadResponse", [
{ no: 1, name: "ok", kind: "scalar", T: 8 /*ScalarType.BOOL*/ },
{ no: 2, name: "entry_id", kind: "scalar", T: 3 /*ScalarType.INT64*/ }
{ no: 2, name: "entry_id", kind: "scalar", T: 3 /*ScalarType.INT64*/ },
{ no: 3, name: "message", kind: "scalar", T: 9 /*ScalarType.STRING*/ }
]);
}
create(value?: PartialMessage<FinalizeCacheEntryUploadResponse>): 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<FinalizeCacheEntryUploadResponse>(this, message, value);
@@ -354,6 +374,9 @@ class FinalizeCacheEntryUploadResponse$Type extends MessageType<FinalizeCacheEnt
case /* int64 entry_id */ 2:
message.entryId = reader.int64().toString();
break;
case /* string message */ 3:
message.message = reader.string();
break;
default:
let u = options.readUnknownField;
if (u === "throw")
@@ -372,6 +395,9 @@ class FinalizeCacheEntryUploadResponse$Type extends MessageType<FinalizeCacheEnt
/* int64 entry_id = 2; */
if (message.entryId !== "0")
writer.tag(2, WireType.Varint).int64(message.entryId);
/* string message = 3; */
if (message.message !== "")
writer.tag(3, WireType.LengthDelimited).string(message.message);
let u = options.writeUnknownFields;
if (u !== false)
(u == true ? UnknownFieldHandler.onWrite : u)(this.typeName, message, writer);