fix(backups): fix unlink ENOENT (#552)

src/remote-handlers/abstract.js

@@ -203,7 +203,7 @@ export default class RemoteHandlerAbstract {
   }
 
   async unlink (file, {
-    checksum = false
+    checksum = true
   } = {}) {
     if (checksum) {
       this._unlink(`${file}.checksum`)::pCatch(noop)

src/remote-handlers/local.js

@@ -1,14 +1,8 @@
 import fs from 'fs-extra'
-import startsWith from 'lodash/startsWith'
+import { dirname, resolve } from 'path'
-import {
+import { noop, startsWith } from 'lodash'
-  dirname,
-  resolve
-} from 'path'
 
 import RemoteHandlerAbstract from './abstract'
-import {
-  noop
-} from '../utils'
 
 export default class LocalHandler extends RemoteHandlerAbstract {
   get type () {
@@ -79,7 +73,12 @@ export default class LocalHandler extends RemoteHandlerAbstract {
   }
 
   async _unlink (file) {
-    return fs.unlink(this._getFilePath(file))
+    return fs.unlink(this._getFilePath(file)).catch(error => {
+      // do not throw if the file did not exist
+      if (error == null || error.code !== 'ENOENT') {
+        throw error
+      }
+    })
   }
 
   async _getSize (file) {

src/xo-mixins/backups.js

@@ -18,6 +18,7 @@ import {
   findIndex,
   includes,
   once,
+  range,
   sortBy,
   startsWith,
   trim
@@ -28,6 +29,7 @@ import vhdMerge, { chainVhd } from '../vhd-merge'
 import xapiObjectToXo from '../xapi-object-to-xo'
 import { lvs, pvs } from '../lvm'
 import {
+  asyncMap,
   forEach,
   mapFilter,
   mapToArray,
@@ -478,8 +480,8 @@ export default class {
 
     const getPath = (file, dir) => dir ? `${dir}/${file}` : file
 
-    await Promise.all(
+    await asyncMap(backups.slice(0, n), backup =>
-      mapToArray(backups.slice(0, n), async backup => /* await */ handler.unlink(getPath(backup, dir)))
+      handler.unlink(getPath(backup, dir))
     )
   }
 
@@ -617,11 +619,9 @@ export default class {
     const fullBackupId = j
 
     // Remove old backups before the most recent full.
-    if (j > 0) {
+    await asyncMap(range(0, j), i =>
-      for (j--; j >= 0; j--) {
+      handler.unlink(`${dir}/${backups[i]}`)
-        await handler.unlink(`${dir}/${backups[j]}`, { checksum: true })
+    )
-      }
-    }
 
     const parent = `${dir}/${backups[fullBackupId]}`
 
@@ -636,7 +636,7 @@ export default class {
         throw e
       }
 
-      await handler.unlink(backup, { checksum: true })
+      await handler.unlink(backup)
     }
 
     // Rename the first old full backup to the new full backup.
@@ -717,7 +717,7 @@ export default class {
       ])
     } catch (error) {
       // Remove new backup. (corrupt).
-      await handler.unlink(backupFullPath, { checksum: true })::pCatch(noop)
+      await handler.unlink(backupFullPath)::pCatch(noop)
 
       throw error
     }
@@ -734,16 +734,14 @@ export default class {
     const nOldBackups = backups.length - retention
 
     if (nOldBackups > 0) {
-      await Promise.all(
+      await asyncMap(backups.slice(0, nOldBackups), backup => Promise.all([
-        mapToArray(backups.slice(0, nOldBackups), async backup => {
+        // Remove json file.
-          // Remove json file.
+        handler.unlink(`${dir}/${backup}`),
-          await handler.unlink(`${dir}/${backup}`)
 
-          // Remove xva file.
+        // Remove xva file.
-          // Version 0.0.0 (Legacy) Delta Backup.
+        // Version 0.0.0 (Legacy) Delta Backup.
-          handler.unlink(`${dir}/${getDeltaBackupNameWithoutExt(backup)}.xva`)::pCatch(noop)
+        handler.unlink(`${dir}/${getDeltaBackupNameWithoutExt(backup)}.xva`)::pCatch(noop)
-        })
+      ]))
-      )
     }
   }
 
@@ -835,13 +833,8 @@ export default class {
       }
     }
 
-    $onFailure(() => Promise.all(
+    $onFailure(() => asyncMap(fulFilledVdiBackups, vdiBackup =>
-      mapToArray(fulFilledVdiBackups, vdiBackup =>
+      handler.unlink(`${dir}/${vdiBackup.value()}`)::pCatch(noop)
-        handler.unlink(
-          `${dir}/${vdiBackup.value()}`,
-          { checksum: true }
-        )::pCatch(noop)
-      )
     ))
 
     if (error) {