diff --git a/lib/tasks/download-assets.js b/lib/tasks/download-assets.js index d38f3c2a..2039e08b 100644 --- a/lib/tasks/download-assets.js +++ b/lib/tasks/download-assets.js @@ -1,5 +1,5 @@ import Promise from 'bluebird' -import { getEntityName } from 'contentful-batch-libs' +import { getEntityName, logEmitter } from 'contentful-batch-libs' import figures from 'figures' import { createWriteStream, promises as fs } from 'fs' import path from 'path' @@ -7,17 +7,23 @@ import { pipeline } from 'stream' import { promisify } from 'util' import { calculateExpiryTimestamp, isEmbargoedAsset, signUrl } from '../utils/embargoedAssets' import axios from 'axios' +import axiosRetry from 'axios-retry' const streamPipeline = promisify(pipeline) +// For streaming responses, axios timeout applies to time-to-first-byte, not total transfer duration +const DEFAULT_DOWNLOAD_TIMEOUT_MS = 30_000 +const DEFAULT_RETRY_LIMIT = 3 + /** * @param {Object} options - The options for downloading the asset. * @param {string} options.url - The URL of the asset to download. * @param {string} options.directory - The directory where the asset should be saved. * @param {import('axios').AxiosInstance} options.httpClient - The HTTP client to use for downloading the asset. + * @param {string} options.assetId - The ID of the asset being downloaded, used in error messages. */ -async function downloadAsset ({ url, directory, httpClient }) { -// handle urls without protocol +async function downloadAsset({ url, directory, httpClient, assetId }) { + // handle urls without protocol if (url.startsWith('//')) { url = 'https:' + url } @@ -46,11 +52,14 @@ async function downloadAsset ({ url, directory, httpClient }) { * @type {import('axios').AxiosError} */ const axiosError = e - throw new Error(`error response status: ${axiosError.response.status}`) + if (axiosError.response) { + throw new Error(`error downloading asset ${assetId} (${url}): HTTP ${axiosError.response.status} ${axiosError.response.statusText}`, { cause: axiosError }) + } + throw new Error(`error downloading asset ${assetId} (${url}): ${e.message}`, { cause: e }) } } -export default function downloadAssets (options) { +export default function downloadAssets(options) { return (ctx, task) => { let successCount = 0 let warningCount = 0 @@ -58,12 +67,24 @@ export default function downloadAssets (options) { const httpClient = axios.create({ headers: options.headers, - timeout: options.timeout, + timeout: options.timeout ?? DEFAULT_DOWNLOAD_TIMEOUT_MS, httpAgent: options.httpAgent, httpsAgent: options.httpsAgent, proxy: options.proxy }) + axiosRetry(httpClient, { + retries: options.retryLimit ?? DEFAULT_RETRY_LIMIT, + retryDelay: axiosRetry.exponentialDelay, + shouldResetTimeout: true, + retryCondition: (error) => + axiosRetry.isNetworkOrIdempotentRequestError(error) || error.response?.status === 429, + onRetry: (retryCount, error) => { + const status = error.response ? `HTTP ${error.response.status}` : (error.code || error.message) + logEmitter.emit('warning', `Asset download failed (${status}), retrying (attempt ${retryCount})...`) + } + }) + return Promise.map(ctx.data.assets, (asset) => { const entityName = getEntityName(asset) if (!asset.fields.file) { @@ -81,14 +102,15 @@ export default function downloadAssets (options) { return Promise.resolve() } - let startingPromise = Promise.resolve({ url, directory: options.exportDir, httpClient }) + const assetId = asset.sys.id + let startingPromise = Promise.resolve({ url, directory: options.exportDir, httpClient, assetId }) if (isEmbargoedAsset(url)) { const { host, accessToken, spaceId, environmentId } = options const expiresAtMs = calculateExpiryTimestamp() startingPromise = signUrl(host, accessToken, spaceId, environmentId, url, expiresAtMs, httpClient) - .then((signedUrl) => ({ url: signedUrl, directory: options.exportDir, httpClient })) + .then((signedUrl) => ({ url: signedUrl, directory: options.exportDir, httpClient, assetId })) } return startingPromise @@ -98,6 +120,7 @@ export default function downloadAssets (options) { successCount++ }) .catch((error) => { + logEmitter.emit('warning', error.message) task.output = `${figures.cross} error downloading ${url}: ${error.message}` errorCount++ }) diff --git a/lib/tasks/get-space-data.js b/lib/tasks/get-space-data.js index 81d21051..69719f1a 100644 --- a/lib/tasks/get-space-data.js +++ b/lib/tasks/get-space-data.js @@ -68,7 +68,8 @@ export default function getFullSourceSpace ({ .then((items) => { ctx.data.tags = items }) - .catch(() => { + .catch((err) => { + logEmitter.emit('warning', `Fetching tags failed — tags will be empty in the export. Error: ${err.message}`) ctx.data.tags = [] }) }), @@ -168,7 +169,7 @@ function getEditorInterfaces (contentTypes) { .catch(() => { // old contentTypes may not have an editor interface but we'll handle in a later stage // but it should not stop getting the data process - logEmitter.emit('warning', `No editor interface found for ${contentType}`) + logEmitter.emit('warning', `No editor interface found for ${contentType.name || contentType.sys.id}`) return Promise.resolve(null) }) }, { diff --git a/package-lock.json b/package-lock.json index fd36abe6..1fc10fb6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "axios": "^1.13.5", + "axios-retry": "^4.5.0", "bfj": "^9.1.3", "bluebird": "^3.3.3", "cli-table3": "^0.6.0", @@ -4780,6 +4781,18 @@ "proxy-from-env": "^2.1.0" } }, + "node_modules/axios-retry": { + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/axios-retry/-/axios-retry-4.5.0.tgz", + "integrity": "sha512-aR99oXhpEDGo0UuAlYcn2iGRds30k366Zfa05XWScR9QaQD4JYiP3/1Qt1u7YlefUOK+cn0CcwoL1oefavQUlQ==", + "license": "Apache-2.0", + "dependencies": { + "is-retry-allowed": "^2.2.0" + }, + "peerDependencies": { + "axios": "0.x || 1.x" + } + }, "node_modules/axios/node_modules/agent-base": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-6.0.2.tgz", @@ -9890,6 +9903,18 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/is-retry-allowed": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/is-retry-allowed/-/is-retry-allowed-2.2.0.tgz", + "integrity": "sha512-XVm7LOeLpTW4jV19QSH38vkswxoLud8sQ57YwJVTPWdiaI9I8keEhGFpBlslyVsgdQy4Opg8QOLb8YRgsyZiQg==", + "license": "MIT", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/is-set": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/is-set/-/is-set-2.0.3.tgz", @@ -17657,16 +17682,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/undici": { - "version": "7.28.0", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.28.0.tgz", - "integrity": "sha512-cRZYrTDwWznlnRiPjggAGxZXanty6M8RV1ff8Wm4LWXBp7/IG8v5DnOm74DtUBp9OONpK75YlPnIjQqX0dBDtA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=20.18.1" - } - }, "node_modules/undici-types": { "version": "6.20.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.20.0.tgz", @@ -18208,4 +18223,4 @@ } } } -} \ No newline at end of file +} diff --git a/package.json b/package.json index 45eec546..fe862851 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ }, "dependencies": { "axios": "^1.13.5", + "axios-retry": "^4.5.0", "bfj": "^9.1.3", "bluebird": "^3.3.3", "cli-table3": "^0.6.0", @@ -147,4 +148,4 @@ "cross-spawn": "^7.0.6", "undici": "^6.27.0" } -} \ No newline at end of file +} diff --git a/test/unit/tasks/download-assets.test.js b/test/unit/tasks/download-assets.test.js index 0024083f..ea6fe6af 100644 --- a/test/unit/tasks/download-assets.test.js +++ b/test/unit/tasks/download-assets.test.js @@ -35,7 +35,7 @@ let output nock(`https:${BASE_PATH}`) .get(EXISTING_ASSET_URL) - .times(8) + .times(10) .reply(200) nock(`https:${BASE_PATH}`) @@ -67,7 +67,7 @@ nock(`https://${API_HOST}`) .times(1) .reply(200, { policy: POLICY, secret: SECRET }) -function getAssets ({ existing = 0, nonExisting = 0, missingUrl = 0, embargoed = 0, unicodeShort = 0, unicodeLong = 0, differentFilename = 0 } = {}) { +function getAssets({ existing = 0, nonExisting = 0, missingUrl = 0, embargoed = 0, unicodeShort = 0, unicodeLong = 0, differentFilename = 0 } = {}) { const existingUrl = `${BASE_PATH}${EXISTING_ASSET_URL}` const embargoedUrl = `${BASE_PATH_SECURE}${EMBARGOED_ASSET_URL}` const nonExistingUrl = `${BASE_PATH}${NON_EXISTING_URL}` @@ -329,7 +329,7 @@ test('it doesn\'t use fileStack url as fallback for the file url and throws a wa const missingUrlsOutputCount = output.mock.calls.filter(call => call[0]?.endsWith('asset.fields.file[en-US].url') || - call[0]?.endsWith('asset.fields.file[de-DE].url')) + call[0]?.endsWith('asset.fields.file[de-DE].url')) expect(missingUrlsOutputCount).toHaveLength(2) }) @@ -389,6 +389,27 @@ test('Downloads assets with long Unicode filenames', () => { }) }) +test('Respects user-supplied timeout option', () => { + const task = downloadAssets({ + exportDir: tmpDirectory, + timeout: 60_000 + }) + const ctx = { + data: { + assets: [...getAssets({ existing: 1 })] + } + } + + return task(ctx, taskProxy) + .then(() => { + expect(ctx.assetDownloads).toEqual({ + successCount: 2, + warningCount: 0, + errorCount: 0 + }) + }) +}) + test('Downloads assets with different filename than URL path', () => { const task = downloadAssets({ exportDir: tmpDirectory