0

EDIT: I have since started using the regular request library for piping which makes the errors handle fine. With that said, this leaves my only issue being the rare occasion where the program does not exit properly and requires manual termination. I believe this post is the same problem I am experiencing now.

I have a program that downloads images from urls obtained through a JSON file. On large requests (80+ images) I will get errors on a couple files such as ETIMEDOUT and Socket hangup.

I'm trying to figure out where exactly I need to catch these unhandled promise errors being thrown as they make my program not exit correctly sometimes forcing the user to Ctrl+C terminate the program.

request({ uri: url, json: true })
.then((data) => {
    const fileArray = [];
    data.posts.forEach((el) => {
        if (!el.filepath) return;
        fileArray.push(el.filepath);
    });
    return fileArray;
})
.then((arr) => {
    arr.forEach(el => {
        request.head(el, () => {
            request({ url: el, encoding: null, forever: true })
                .pipe(createWriteStream(el))
                .on('close', () => { console.log('File Downloaded!'); })
                .on('error', (e) => { console.log(e); });
        });
    });
})
.catch((error) => console.log(error));

The first request block of code should be irrelevant since the errors are on file download NOT on requesting the json data. The .pipe() has a .on 'error' handler attached to it. request.head() is not "then-able" to my knowledge and surrounding the entire forEach loop in a try/catch doesn't work either.

I'm not exactly sure how to get around these errors but for now the program runs fine enough that I just want to make sure I can catch any errors for now to improve user experience.

  • Where exactly are you getting an unhandled promise rejection, with what error message and stacktrace? Also, which of these functions produces a promise, what is `request`? – Bergi Jan 29 '19 at 18:31
  • "*surrounding the entire `forEach` loop in a try/catch doesn't work either.*" - indeed, [don't use `forEach` with promises](https://stackoverflow.com/q/37576685/1048572) – Bergi Jan 29 '19 at 18:34
  • `(node:7160) UnhandledPromiseRejectionWarning: RequestError: Error: socket hang up at new RequestError` The errors I get don't give me a clear indication of where it occurs, however because the errors happen during the downloading portion its safe to assume I'm handling the download loop incorrectly – TheBetterEnvy Jan 29 '19 at 23:17

1 Answers1

0

I would push it into a promise array, then call promise.all.

var filesToProcess = [];
request({ uri: url, json: true })
.then((data) => {
    const fileArray = [];
    data.posts.forEach((el) => {
        if (!el.filepath) return;
        fileArray.push(el.filepath);
    });
    return fileArray;
})
.then((arr) => {
    arr.forEach(el => {
        var promise = new Promise(function(resolve, reject) {  
             request.head(el, () => {
                 request({ url: el, encoding: null, forever: true })
                    .pipe(createWriteStream(el))
                    .on('close', () => { resolve('File Downloaded!'); })
                    .on('error', (e) => { reject(e); });
        });
        filesToProcess.push(promise);  
    })


    });
})
.catch((error) => console.log(error));

Promise.all(filesToProcess).then(values => {    
    //all of the promises successful
}).catch(err => {
   //one of the promises failed
});
Matt Kuhns
  • 1,328
  • 1
  • 13
  • 26
  • The `Promise.all` call would definitely need to go inside the `then` callback where the array is created and filled. Also you probably should avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) if the `request` call already creates a promise by itself. – Bergi Jan 29 '19 at 18:33
  • This is very weird. I implemented this code taking Bergi's suggestion to move the `Promise.all` call(and obviously ignoring the antipattern bit) and the result was... inconclusive? If I attach a `then/catch` to `Promise.all` I get the expected amount of unhandled promise errors but if I don't attach the `then/catch` it doesn't error **at all** even when downloading 200 images... Which is nice from a performance perspective but its really making it hard to properly catch errors haha – TheBetterEnvy Jan 29 '19 at 23:40
  • Post your solution as an answer. I'm curious to see what you have. Perhaps you could push a file in the array that you know will fail. – Matt Kuhns Jan 30 '19 at 16:33