Skip to content

Commit

Permalink
Graceful handling of error (non-validation one) (#1122)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@tiwarishub
tiwarishub committed Jun 24, 2022
1 parent 9b7bcb1 commit 46231a7
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 30 deletions.
8 changes: 6 additions & 2 deletions packages/cache/__tests__/restoreCache.test.ts
View file
Expand Up @@ -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'
)
})

Expand Down
42 changes: 31 additions & 11 deletions packages/cache/__tests__/saveCache.test.ts
View file
Expand Up @@ -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
Expand All @@ -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'
Expand All @@ -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
Expand Down Expand Up @@ -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'
Expand All @@ -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
Expand All @@ -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'
Expand All @@ -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')
Expand All @@ -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
Expand All @@ -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')
Expand All @@ -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
Expand Down
54 changes: 37 additions & 17 deletions packages/cache/src/cache.ts
View file
Expand Up @@ -86,23 +86,24 @@ export async function restoreCache(
}

const compressionMethod = await utils.getCompressionMethod()
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
}

// 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}`)
archivePath = path.join(
await utils.createTempDirectory(),
utils.getCacheFileName(compressionMethod)
)
core.debug(`Archive Path: ${archivePath}`)

try {
// Download the cache from the cache entry
await cacheHttpClient.downloadCache(
cacheEntry.archiveLocation,
Expand All @@ -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 {
Expand All @@ -132,7 +143,7 @@ export async function restoreCache(
}
}

return cacheEntry.cacheKey
return undefined
}

/**
Expand All @@ -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:')
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 46231a7

Please sign in to comment.