3

When the below code errors (ping() rejects its promise), I get the warning. The HTTP function seems to error out just fine. Something must be happening in ping() itself, I guess, which is somehow avoiding the try-catch.

Could someone enlighten me? (This is after a few attempts at changing things to get it working.)

(async () => {
    try {
            let webTask, pingTask;

            try {
                    webTask = httpsGet(urls[0]);
            } catch (e) {
                    console.log(e);
            }

            try {
                    pingTask = ping('8.8.8.8');
            } catch (e) {
                    console.log(e);
            }

            try {
                    const webResult = await webTask;
                    console.log('HTTP result:', webResult);
            } catch (e) {
                    console.log(e);
            }

            try {
                    const pingResult = await pingTask;
                    console.log('Ping result:', pingResult);
            } catch (e) {
                    console.log(e);
            }
    } catch (e) {
            console.log('Error:', e);
    }
})();

The error is:

"main.js" 137 lines, 2945 characters
(node:58299) UnhandledPromiseRejectionWarning: #<Object>
(node:58299) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:58299) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

This is earlier in the file where I define my ping function:

const netping = require('net-ping');
const pingSession = netping.createSession({
    retries: 0,
    timeout: 10000
});

const ping = ip => {
    return new Promise((resolve, reject) => {
            let result = { ipAddress: ip, start: new Date(Date.now()), end: null, error: null, duration_ms: -1 };

            pingSession.pingHost(ip, (error, target) => {
                    result.end = new Date(Date.now());
                    result.duration_ms = result.end - result.start;
                    if (error) {
                            result.error = error;
console.log('rejecting promise');
                            reject(result);
                    } else {
                            resolve(result);
console.log('resolving promise');
                    }
            });
    });
};

NodeJS 11.13.0

Nickolay
  • 31,095
  • 13
  • 107
  • 185
Josh
  • 6,944
  • 8
  • 41
  • 64
  • When the rejection is logged, where does it point to, exactly? Are any errors logged before the rejection? – CertainPerformance Jun 23 '19 at 00:28
  • I'd try https://stackoverflow.com/a/46964348/1026 or reducing further: what makes you think the rejected promise that's not handled is the one returned from `ping()`? Nowhere in the message does it say that. – Nickolay Jun 23 '19 at 00:54
  • Use [`--trace-warnings`](https://nodejs.org/api/cli.html#cli_trace_warnings), it might show more info. – Lawrence Cherone Jun 23 '19 at 01:00
  • Start by changing to `const webResult = await httpsGet(urls[0]);` and `const pingResult = await ping('8.8.8.8');` and get rid of the intermediate promise variables and the duplicated `try/catch` es (which really are not needed). It may be that the interpreter doesn't realize you are handling the rejection later in the code. – jfriend00 Jun 23 '19 at 02:45
  • @jfriend00 That is what I had originally when this happened. – Josh Jun 23 '19 at 14:05
  • @Nickolay If I comment out the ping part, it goes away. – Josh Jun 23 '19 at 14:06
  • @CertainPerformance This is all the output I get for the error. – Josh Jun 23 '19 at 14:06

1 Answers1

0

await is the javascript construct which converts the rejection of the promise to an exception. That is, await is the construct which handles the rejection.

When you write:

try {
    pingTask = ping('8.8.8.8');
} catch (e) {
    console.log(e);
}

There's no await there, so there's nothing to convert the rejection to an exception, or indeed to handle the rejection in any way.

If you're going to call ping() without waiting, then you need a more explicit rejection handling.

EDIT

Here's a minimal reproducer:

function sleep(ms) {
    return new Promise(resolve => setTimeout(resolve, ms));
}

const test = (ms, msg) => {
    return new Promise((resolve, reject) => {
        sleep(ms).then(reject(msg)).catch(console.log);
    });
};

(async () => {
    let task1;

    try {
        const ms = 50;
        task1 = test(ms, "hey");
        await sleep(ms * 2); // otherwise you don't get an error
        await task1;
    } catch (e) {
        console.log(e);
    }
})().then(console.log).catch(console.log);

(node:12822) UnhandledPromiseRejectionWarning: hey (node:12822) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) (node:12822) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. hey undefined (node:12822) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)

The Promise() constructor starts running test(), after 50 ms the promise is rejected but there is nothing to convert the rejection to an exception. If we remove the 100 ms sleep, then await registers its "convert rejection to exception" logic onto the promise way before the 50 ms sleep is done, and so when the promise gets rejected ~49 ms after await was called, there's a handler to convert it to an exception.

root
  • 5,528
  • 1
  • 7
  • 15
  • 1
    So, `pingTask` is a promise. Nothing wrong with that assignment. Then, later in the code, they do `await pingTask` and it is surrounded by `try/catch` which has full error handling. I don't see what you're saying is the problem. – jfriend00 Jun 23 '19 at 07:28
  • @jfriend00 answer edited to elaborate on the issue. – root Jun 23 '19 at 07:50
  • @root Originally I had everything in a single try-catch and single-line `var x = await foo()` lines, and that is when I first received the warning. I broke it up hoping to narrow down what was causing it. So if I understand your explanation, I would still need to call `.catch` on `pingTask`, then throw, to fit into the current mess of try-catches, correct? What I was trying to do is run both tasks at the same time; thinking that `await` blocks. Is the only way to do that (if possible at all) to use the promises directly? Edit: or run in parallel but AFAIK that isn't easily possible. – Josh Jun 23 '19 at 14:16
  • I rewrote it and can see that using promises directly does run concurrently, while `await` blocks, and I now know that `await` converts the rejection into a throw. It doesn't seem like I can use await and concurrency together reliably. I think I understand now, unless someone corrects me. Thanks! – Josh Jun 23 '19 at 14:36
  • 1
    @jfriend00 I think the issue is that, if two Promises exist, and one Promise rejects while the other is being `await`ed, an unhandled rejection will occur *even if* the rejected Promise gets `await`ed inside a `try/catch` later. The interpreter doesn't know that the rejected Promise is going to be handled later, because it's currently `await`ing a different one. – CertainPerformance Jun 23 '19 at 21:42
  • 1
    @CertainPerformance - Which is why using intermediate variables to hold the promise BEFORE a `catch` is protecting the promise can cause problems. I don't write code that way for this reason. It turns out the OP wanted to run these in parallel anyway to the whole serially `await` logic is just the wrong tool for what they wanted to accomplish. They probably want to use `Promise.allSettled()` so they can be notified when both operations are done. – jfriend00 Jun 23 '19 at 23:31