Difference in `download` method - rejected promise on close

[Frando]

I noticed a difference in behavior between hypercore and hypercore-promise:

const hypercore = require('@geut/hypercore-promise')
const ram = require('random-access-memory')
const feed = hypercore(ram)
feed.download({ start: 0, end: -1 })
feed.close()

produces:

(node:709947) UnhandledPromiseRejectionWarning: Error: Feed is closed
    at /home/bit/Code/node/sonar/node_modules/hypercore/index.js:1508:26
    at Request.done [as _callback] (/home/bit/Code/node/sonar/node_modules/hypercore/lib/storage.js:228:5)
    at Request.callback (/home/bit/Code/node/sonar/node_modules/random-access-storage/index.js:173:8)
    at nextTickCallback (/home/bit/Code/node/sonar/node_modules/random-access-storage/index.js:272:7)
    at processTicksAndRejections (internal/process/task_queues.js:85:21)

The reason is that feed.download() in hypercore optionally takes a callback. The callback is called with an Error Feed is closed if the download request is still active when closing a feed. Because the callback is optional, this often is a noop. hypercore-promise appends a callback and returns a promise that would resolve when the download is complete, or is rejected when the feed is closed before the download is complete (which is always the case for infinite downloads).

What works now is to pass null as second argument to feed.download() on a promisified hypercore (original hypercore receives null for callback, thus not calling it with an error) if you don't want to await the result of the download.

I don't know of a clean solution right away. Maybe documenting is enough.

[dpaez]

Hey thanks for the info @Frando! We will try to think for some solution this week 🤔 But if nothing comes up, documenting this would be good as you mention.

👍

[RangerMauve]

Documenting seems like a good idea. With the null example. Are there other methods where the CB is optional?

[tinchoz49]

Hey @Frando, thank you for sharing this issue. I agree of adding this edge case to the documentation, I'm going to check if there is another method with the same problem.