Quellcode durchsuchen

Merge pull request #24142 from overleaf/bg-backup-fix-blob-error-handling

fix bug which caused errors to be ignored when backing up blobs

GitOrigin-RevId: 64674a484a6e22493cba8f8e47d4671072dc2e9a
Brian Gough vor 1 Jahr
Ursprung
Commit
9ff3a45690

+ 2 - 1
services/history-v1/storage/scripts/backup.mjs

@@ -184,7 +184,8 @@ async function backupBlobs(projectId, historyId, blobs, limiter, persistor) {
       limiter(backupSingleBlob, projectId, historyId, blob, tmpDir, persistor)
       limiter(backupSingleBlob, projectId, historyId, blob, tmpDir, persistor)
     )
     )
 
 
-    await Promise.allSettled(blobBackupOperations)
+    // Reject if any blob backup fails
+    await Promise.all(blobBackupOperations)
   } finally {
   } finally {
     if (tmpDir) {
     if (tmpDir) {
       await fs.rm(tmpDir, { recursive: true, force: true })
       await fs.rm(tmpDir, { recursive: true, force: true })

+ 35 - 2
services/history-v1/test/acceptance/js/storage/backup.test.mjs

@@ -585,6 +585,40 @@ describe('backup script', function () {
         expect(JSON.parse(chunkContent)).to.deep.equal(rawHistory)
         expect(JSON.parse(chunkContent)).to.deep.equal(rawHistory)
       }
       }
     })
     })
+
+    it('should throw an error if downloading a blob fails', async function () {
+      const blobStore = new BlobStore(historyId)
+      const blob = await blobStore.putFile(
+        testFiles.path('null_characters.txt')
+      )
+      const change = new Change(
+        [Operation.addFile('broken-file', File.fromHash(blob.getHash()))],
+        new Date(),
+        []
+      )
+      // Persist all changes
+      await persistChanges(historyId, [change], limitsToPersistImmediately, 53)
+
+      // Delete the blob from the underlying storage to simulate a failure
+      const bucket = config.get('blobStore.projectBucket')
+      const key = makeProjectKey(historyId, blob.getHash())
+      await persistor.deleteObject(bucket, key)
+
+      // Run backup script - it should fail because the blob is missing
+      let result
+      try {
+        result = await runBackupScript(['--projectId', projectId])
+        expect.fail('Backup script should have failed')
+      } catch (err) {
+        expect(err).to.exist
+        expect(result).to.not.exist
+      }
+
+      // Verify that backup did not complete
+      const newBackupStatus = await getBackupStatus(projectId)
+      expect(newBackupStatus.backupStatus.lastBackedUpVersion).to.equal(50) // backup fails on final chunk
+      expect(newBackupStatus.currentEndVersion).to.equal(54) // backup is incomplete due to missing blob
+    })
   })
   })
 })
 })
 
 
@@ -617,8 +651,7 @@ async function runBackupScript(args) {
     result = { stdout, stderr, status: code }
     result = { stdout, stderr, status: code }
   }
   }
   if (result.status !== 0) {
   if (result.status !== 0) {
-    console.log(result)
+    throw new Error('backup failed')
   }
   }
-  expect(result.status).to.equal(0)
   return result
   return result
 }
 }