fix: improve error handling and logging for asset downloads and tags [AIS-159]

lib/tasks/download-assets.js

@@ -1,23 +1,29 @@
 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'
 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,24 +52,39 @@ 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
     let errorCount = 0
 
     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++
           })

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)
       })
   }, {

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"
   }
-}
+}

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