Add the result of a chained promise into promise.all()

Question

I try construct a promise, named downloadTextPromise, to download a file and then in Promise.all() to read the file content. However, I still cannot read the file content in Promise.all().then.

I suspect that downloadTextPromise is a two-step promise, so only the first step (HTTP GET) is pushed into promises array. But I have little idea how to fix it.

Any hint? any solution that works is welcoming, request-promise-native package is not compulsory.

My code is as below:

const request = require("request-promise-native");
let promises = [];

//put several promises into promises array;
.....

let downloadTextPromise = request.get("http://example.com/xyz.txt").pipe(fs.createWriteStream(`~/xyz.txt`));

promises.push(downloadTextPromise);
Promise.all(promises).then(() => {
    //I need xyz.txt content at this stage, but it is not ready immediately.
}

but how? I use Promise.all() is meant to wait until all promises are done, now it comes the extra condition that I dont know how to handle — Xi Xiao, Jun 21 '17 at 19:33
@XiXiao Streams and promises are not the same thing. I would say you need to wrap the stream in a promise and manually resolve it when the stream is finished. — Lennholm, Jun 21 '17 at 19:35
@XiXiao That's totally unrelated to the `Promise.all`. You have to find a way to create a `downloadTextPromise` that doesn't fulfill before the file is completely written. — Bergi, Jun 21 '17 at 19:37
Thank you both for showing the direction, I will give it a try. — Xi Xiao, Jun 21 '17 at 19:39

score 0 · Answer 1 · answered Jun 21 '17 at 19:41

0

Firstly, check if downloadTextPromise is actually a promise. The library's documentation doesn't mention how pipe works. If it returns a native promise, you can do console.log(downloadTextPromise instanceof Promise). Otherwise, you can try console.log(!!downloadTextPromise.then).

If it's a promise, then one possibility is that the promise is resolved synchronously after the the stream ends. The code to actually write the file is added to the message queue. In this case, you can fix it by doing:

Promise.all(promises).then(() => {
    setTimeout(() => {
        // Do stuff with xyz.txt
    }, 0);
}

answered Jun 21 '17 at 19:41

Leo Jiang

24,497
49
154
284

`console.log(downloadTextPromise instanceof Promise)` actually returns `false`. Thus I should wrap it into a function that returns a promise? – Xi Xiao Jun 21 '17 at 19:50
Yep that's correct. I don't know what kind of value `pipe` returns, but I'd imagine it has a way to listen for when it finishes (maybe an `on` method or something like `oncomplete`). Wrap the pipe in a promise and resolve the promise when the pipe finishes. – Leo Jiang Jun 21 '17 at 19:55
@XiXiao Better test for `typeof downloadTextPromise.then`, not all promises are instances of the native `Promise` implementation. – Bergi Jun 21 '17 at 20:10
@LeoJiang Uh, `setTimeout` does not help. The writing of the file is not a synchronous process that is queued, it's a background process that keeps going *while* the event loop keeps spinning. – Bergi Jun 21 '17 at 20:11
1

@Bergi If I add `then()` after `pipe()`, it throws error `no such function`. I assume pipe does not return any type of promises. – Xi Xiao Jun 21 '17 at 20:23
OK, then this is settled. – Bergi Jun 21 '17 at 20:37
@Bergi, I made a solution and posted as one answer. It, however, introduces a new problem... – Xi Xiao Jun 21 '17 at 20:40

score 0 · Answer 2 · edited Nov 02 '17 at 23:58

0

The issue is the writeStream part, so I should build a function returning correct promise.

I don't find the way to construct one returning a promise from request.get().pipe(), instead I use request.get().then() and fs.writeFile() as below.

However, Another problem appears from the new solution: This fs.writeFile works all OK for txt files; for mp3 or mp4, the function returns no error, but the file content is invalid for further processing in Promise.all().then().

function downloadTextPromise() {
    return new Promise((resolve, reject) => {
        rp.get("http://example.com/xyz.txt").then(data => {
          fs.writeFile("./xyz.txt, data, err => {
            if (err) reject(err);
            resolve(true);
          });
        });
    });
}

edited Nov 02 '17 at 23:58

000

26,951
10
71
101

answered Jun 21 '17 at 20:38

Xi Xiao

953
1
12
28

Is `data` a string? Does it use default file encoding (utf8)? Probably you want to tell `request` to download a `Buffer` – Bergi Jun 21 '17 at 21:03
You should place the `new Promise` call inside the `then` callback – Bergi Jun 21 '17 at 21:03
Could you tell why I should put the call inside the `then`? Simply be closer to the callback where resolve and reject will take place? What is the flaw with my current code? Thanks. – Xi Xiao Jun 22 '17 at 08:01
Yes, always put it as close as possible to the callback. It's an instance of the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) - and it doesn't handle rejections of the `rp.get(…)` promise – Bergi Jun 22 '17 at 08:06

Add the result of a chained promise into promise.all()

2 Answers2