fix: remove disposer

promises have finally. actually

Author: wraithgar

lib/content/write.js

@@ -13,34 +13,37 @@ const path = require('path')
 const rimraf = util.promisify(require('rimraf'))
 const ssri = require('ssri')
 const uniqueFilename = require('unique-filename')
-const { disposer } = require('./../util/disposer')
 const fsm = require('fs-minipass')
 
 const writeFile = util.promisify(fs.writeFile)
 
 module.exports = write
 
-function write (cache, data, opts = {}) {
+async function write (cache, data, opts = {}) {
   const { algorithms, size, integrity } = opts
   if (algorithms && algorithms.length > 1) {
     throw new Error('opts.algorithms only supports a single algorithm for now')
   }
 
   if (typeof size === 'number' && data.length !== size) {
-    return Promise.reject(sizeError(size, data.length))
+    throw sizeError(size, data.length)
   }
 
   const sri = ssri.fromData(data, algorithms ? { algorithms } : {})
   if (integrity && !ssri.checkData(data, integrity, opts)) {
-    return Promise.reject(checksumError(integrity, sri))
+    throw checksumError(integrity, sri)
   }
 
-  return disposer(makeTmp(cache, opts), makeTmpDisposer,
-    (tmp) => {
-      return writeFile(tmp.target, data, { flag: 'wx' })
-        .then(() => moveToDestination(tmp, cache, sri, opts))
-    })
-    .then(() => ({ integrity: sri, size: data.length }))
+  const tmp = await makeTmp(cache, opts)
+  try {
+    await writeFile(tmp.target, data, { flag: 'wx' })
+    await moveToDestination(tmp, cache, sri, opts)
+    return { integrity: sri, size: data.length }
+  } finally {
+    if (!tmp.moved) {
+      await rimraf(tmp.target)
+    }
+  }
 }
 
 module.exports.stream = writeStream
@@ -94,18 +97,22 @@ function writeStream (cache, opts = {}) {
   return new CacacheWriteStream(cache, opts)
 }
 
-function handleContent (inputStream, cache, opts) {
-  return disposer(makeTmp(cache, opts), makeTmpDisposer, (tmp) => {
-    return pipeToTmp(inputStream, cache, tmp.target, opts)
-      .then((res) => {
-        return moveToDestination(
-          tmp,
-          cache,
-          res.integrity,
-          opts
-        ).then(() => res)
-      })
-  })
+async function handleContent (inputStream, cache, opts) {
+  const tmp = await makeTmp(cache, opts)
+  try {
+    const res = await pipeToTmp(inputStream, cache, tmp.target, opts)
+    await moveToDestination(
+      tmp,
+      cache,
+      res.integrity,
+      opts
+    )
+    return res
+  } finally {
+    if (!tmp.moved) {
+      await rimraf(tmp.target)
+    }
+  }
 }
 
 function pipeToTmp (inputStream, cache, tmpTarget, opts) {
@@ -136,11 +143,7 @@ function pipeToTmp (inputStream, cache, tmpTarget, opts) {
     outStream
   )
 
-  return pipeline.promise()
-    .then(() => ({ integrity, size }))
-    .catch(er => rimraf(tmpTarget).then(() => {
-      throw er
-    }))
+  return pipeline.promise().then(() => ({ integrity, size }))
 }
 
 function makeTmp (cache, opts) {
@@ -151,14 +154,6 @@ function makeTmp (cache, opts) {
   }))
 }
 
-function makeTmpDisposer (tmp) {
-  if (tmp.moved) {
-    return Promise.resolve()
-  }
-
-  return rimraf(tmp.target)
-}
-
 function moveToDestination (tmp, cache, sri, opts) {
   const destination = contentPath(cache, sri)
   const destDir = path.dirname(destination)

lib/entry-index.js

@@ -8,7 +8,6 @@ const path = require('path')
 const ssri = require('ssri')
 const uniqueFilename = require('unique-filename')
 
-const { disposer } = require('./util/disposer')
 const contentPath = require('./content/path')
 const fixOwner = require('./util/fix-owner')
 const hashToSegments = require('./util/hash-to-segments')
@@ -102,7 +101,12 @@ async function compact (cache, key, matchFn, opts = {}) {
   }
 
   // write the file atomically
-  await disposer(setup(), teardown, write)
+  const tmp = await setup()
+  try {
+    await write(tmp)
+  } finally {
+    await teardown(tmp)
+  }
 
   // we reverse the list we generated such that the newest
   // entries come first in order to make looping through them easier

lib/util/disposer.js

@@ -209,7 +209,7 @@ t.test('cleans up tmp on successful completion', (t) => {
     }))
 })
 
-t.test('cleans up tmp on error', (t) => {
+t.test('cleans up tmp on streaming error', (t) => {
   const CONTENT = 'foobarbaz'
   const CACHE = t.testdir()
   return t.rejects(
@@ -233,6 +233,25 @@ t.test('cleans up tmp on error', (t) => {
     }))
 })
 
+t.test('cleans up tmp on non streaming error', (t) => {
+  // mock writefile and make it reject
+  const CONTENT = 'foobarbaz'
+  const CACHE = t.testdir({ 'content-v2': 'oh no a file' })
+  return t.rejects(write(CACHE, CONTENT))
+    .then(() => new Promise((resolve, reject) => {
+      const tmp = path.join(CACHE, 'tmp')
+      fs.readdir(tmp, function (err, files) {
+        if (!err || (err && err.code === 'ENOENT')) {
+          files = files || []
+          t.same(files, [], 'nothing in the tmp dir!')
+          resolve()
+        } else {
+          reject(err)
+        }
+      })
+    }))
+})
+
 t.test('checks the size of stream data if opts.size provided', (t) => {
   const CONTENT = 'foobarbaz'
   let int1 = null
@@ -270,12 +289,11 @@ t.test('checks the size of stream data if opts.size provided', (t) => {
   })
 })
 
-t.test('only one algorithm for now', t => {
+t.test('only one algorithm for now', async t => {
   const CACHE = t.testdir()
-  t.throws(() => write(CACHE, 'foo', { algorithms: [1, 2] }), {
+  await t.rejects(() => write(CACHE, 'foo', { algorithms: [1, 2] }), {
     message: 'opts.algorithms only supports a single algorithm for now',
   })
-  t.end()
 })
 
 t.test('writes to cache with default options', t => {