1
0
Fork 0

Graceful handling of error (non-validation one) (#1122)

* Initial changes

* added info error as well

* Format

* Unused package

* adding message field

* removed line

* Review comments

* review comment to add validation as errors handling
pull/1123/head
Shubham Tiwari 2022-06-24 10:46:20 +05:30 committed by GitHub
parent 9b7bcb1567
commit 46231a7da3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 75 additions and 31 deletions

View File

@ -73,13 +73,17 @@ test('restore with no cache found', async () => {
test('restore with server error should fail', async () => {
const paths = ['node_modules']
const key = 'node-test'
const logWarningMock = jest.spyOn(core, 'warning')
jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(() => {
throw new Error('HTTP Error Occurred')
})
await expect(restoreCache(paths, key)).rejects.toThrowError(
'HTTP Error Occurred'
const cacheKey = await restoreCache(paths, key)
expect(cacheKey).toBe(undefined)
expect(logWarningMock).toHaveBeenCalledTimes(1)
expect(logWarningMock).toHaveBeenCalledWith(
'Failed to restore: HTTP Error Occurred'
)
})

View File

@ -48,6 +48,7 @@ test('save with large cache outputs should fail', async () => {
const cachePaths = [path.resolve(filePath)]
const createTarMock = jest.spyOn(tar, 'createTar')
const logWarningMock = jest.spyOn(core, 'warning')
const cacheSize = 11 * 1024 * 1024 * 1024 //~11GB, over the 10GB limit
jest
@ -58,8 +59,11 @@ test('save with large cache outputs should fail', async () => {
.spyOn(cacheUtils, 'getCompressionMethod')
.mockReturnValueOnce(Promise.resolve(compression))
await expect(saveCache([filePath], primaryKey)).rejects.toThrowError(
'Cache size of ~11264 MB (11811160064 B) is over the 10GB limit, not saving cache.'
const cacheId = await saveCache([filePath], primaryKey)
expect(cacheId).toBe(-1)
expect(logWarningMock).toHaveBeenCalledTimes(1)
expect(logWarningMock).toHaveBeenCalledWith(
'Failed to save: Cache size of ~11264 MB (11811160064 B) is over the 10GB limit, not saving cache.'
)
const archiveFolder = '/foo/bar'
@ -79,6 +83,7 @@ test('save with large cache outputs should fail in GHES with error message', asy
const cachePaths = [path.resolve(filePath)]
const createTarMock = jest.spyOn(tar, 'createTar')
const logWarningMock = jest.spyOn(core, 'warning')
const cacheSize = 11 * 1024 * 1024 * 1024 //~11GB, over the 10GB limit
jest
@ -106,8 +111,11 @@ test('save with large cache outputs should fail in GHES with error message', asy
return response
})
await expect(saveCache([filePath], primaryKey)).rejects.toThrowError(
'The cache filesize must be between 0 and 1073741824 bytes'
const cacheId = await saveCache([filePath], primaryKey)
expect(cacheId).toBe(-1)
expect(logWarningMock).toHaveBeenCalledTimes(1)
expect(logWarningMock).toHaveBeenCalledWith(
'Failed to save: The cache filesize must be between 0 and 1073741824 bytes'
)
const archiveFolder = '/foo/bar'
@ -127,6 +135,7 @@ test('save with large cache outputs should fail in GHES without error message',
const cachePaths = [path.resolve(filePath)]
const createTarMock = jest.spyOn(tar, 'createTar')
const logWarningMock = jest.spyOn(core, 'warning')
const cacheSize = 11 * 1024 * 1024 * 1024 //~11GB, over the 10GB limit
jest
@ -150,8 +159,11 @@ test('save with large cache outputs should fail in GHES without error message',
return response
})
await expect(saveCache([filePath], primaryKey)).rejects.toThrowError(
'Cache size of ~11264 MB (11811160064 B) is over the data cap limit, not saving cache.'
const cacheId = await saveCache([filePath], primaryKey)
expect(cacheId).toBe(-1)
expect(logWarningMock).toHaveBeenCalledTimes(1)
expect(logWarningMock).toHaveBeenCalledWith(
'Failed to save: Cache size of ~11264 MB (11811160064 B) is over the data cap limit, not saving cache.'
)
const archiveFolder = '/foo/bar'
@ -168,6 +180,7 @@ test('save with large cache outputs should fail in GHES without error message',
test('save with reserve cache failure should fail', async () => {
const paths = ['node_modules']
const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
const logInfoMock = jest.spyOn(core, 'info')
const reserveCacheMock = jest
.spyOn(cacheHttpClient, 'reserveCache')
@ -187,9 +200,13 @@ test('save with reserve cache failure should fail', async () => {
.spyOn(cacheUtils, 'getCompressionMethod')
.mockReturnValueOnce(Promise.resolve(compression))
await expect(saveCache(paths, primaryKey)).rejects.toThrowError(
`Unable to reserve cache with key ${primaryKey}, another job may be creating this cache.`
const cacheId = await saveCache(paths, primaryKey)
expect(cacheId).toBe(-1)
expect(logInfoMock).toHaveBeenCalledTimes(1)
expect(logInfoMock).toHaveBeenCalledWith(
`Failed to save: Unable to reserve cache with key ${primaryKey}, another job may be creating this cache. More details: undefined`
)
expect(reserveCacheMock).toHaveBeenCalledTimes(1)
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, paths, {
compressionMethod: compression
@ -203,7 +220,7 @@ test('save with server error should fail', async () => {
const filePath = 'node_modules'
const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
const cachePaths = [path.resolve(filePath)]
const logWarningMock = jest.spyOn(core, 'warning')
const cacheId = 4
const reserveCacheMock = jest
.spyOn(cacheHttpClient, 'reserveCache')
@ -228,9 +245,12 @@ test('save with server error should fail', async () => {
.spyOn(cacheUtils, 'getCompressionMethod')
.mockReturnValueOnce(Promise.resolve(compression))
await expect(saveCache([filePath], primaryKey)).rejects.toThrowError(
'HTTP Error Occurred'
await saveCache([filePath], primaryKey)
expect(logWarningMock).toHaveBeenCalledTimes(1)
expect(logWarningMock).toHaveBeenCalledWith(
'Failed to save: HTTP Error Occurred'
)
expect(reserveCacheMock).toHaveBeenCalledTimes(1)
expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, [filePath], {
compressionMethod: compression

View File

@ -86,23 +86,24 @@ export async function restoreCache(
}
const compressionMethod = await utils.getCompressionMethod()
// path are needed to compute version
const cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, {
compressionMethod
})
if (!cacheEntry?.archiveLocation) {
// Cache not found
return undefined
}
const archivePath = path.join(
await utils.createTempDirectory(),
utils.getCacheFileName(compressionMethod)
)
core.debug(`Archive Path: ${archivePath}`)
let archivePath = ''
try {
// path are needed to compute version
const cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, {
compressionMethod
})
if (!cacheEntry?.archiveLocation) {
// Cache not found
return undefined
}
archivePath = path.join(
await utils.createTempDirectory(),
utils.getCacheFileName(compressionMethod)
)
core.debug(`Archive Path: ${archivePath}`)
// Download the cache from the cache entry
await cacheHttpClient.downloadCache(
cacheEntry.archiveLocation,
@ -123,6 +124,16 @@ export async function restoreCache(
await extractTar(archivePath, compressionMethod)
core.info('Cache restored successfully')
return cacheEntry.cacheKey
} catch (error) {
const typedError = error as Error
if (typedError.name === ValidationError.name) {
throw error
} else {
// Supress all non-validation cache related errors because caching should be optional
core.warning(`Failed to restore: ${(error as Error).message}`)
}
} finally {
// Try to delete the archive to save space
try {
@ -132,7 +143,7 @@ export async function restoreCache(
}
}
return cacheEntry.cacheKey
return undefined
}
/**
@ -152,7 +163,7 @@ export async function saveCache(
checkKey(key)
const compressionMethod = await utils.getCompressionMethod()
let cacheId = null
let cacheId = -1
const cachePaths = await utils.resolvePaths(paths)
core.debug('Cache Paths:')
@ -217,6 +228,15 @@ export async function saveCache(
core.debug(`Saving Cache (ID: ${cacheId})`)
await cacheHttpClient.saveCache(cacheId, archivePath, options)
} catch (error) {
const typedError = error as Error
if (typedError.name === ValidationError.name) {
throw error
} else if (typedError.name === ReserveCacheError.name) {
core.info(`Failed to save: ${typedError.message}`)
} else {
core.warning(`Failed to save: ${typedError.message}`)
}
} finally {
// Try to delete the archive to save space
try {