Getting UnhandledPromiseRejectionWarning though catching rejections as good as possible

Question

I'm aware of existing answers to issues involving UnhandledPromiseRejectionWarning on SO, but none of the found ones seems to be matching my case sufficiently. Basically, I thought I know how to handle promises. But this case is driving me nuts and so I'd like to ask for some support here: How to get rid of that warning?

I am implementing some writable stream in NodeJS with the following implementation of _write():

_write( data, encoding, doneFn ) {
    Promise.race( [
        this.getSocket(),
        new Promise( ( resolve, reject ) => {
            setTimeout( reject, 1000, Object.assign( new Error( `timeout on trying to connect w/ ${this.address}` ), { code: "ECONNREFUSED" } ) );
        } ),
    ] )
        .then( socket => {
            if ( socket ) {
                return new Promise( ( resolve, reject ) => {
                    socket.once( "error", reject );
                    socket.write( data, encoding, () => {
                        socket.removeListener( "error", reject );

                        resolve();
                    } );
                } );
            }
        } )
        .then( doneFn )
        .catch( error => {
            const { address } = this;

            switch ( error.code ) {
                case "EPIPE" :
                case "ECONNRESET" :
                    Log( `lost connection w/ ${address.id}` );
                    break;

                case "ECONNREFUSED" :
                    Log( `failed to connect w/ ${address.id}` );
                    break;
            }

            console.log( "error is", error.message );
            doneFn( error );
        } );
}

When I test this code for properly emitting error on timeout I get the mysterious UnhandledPromiseRejectionWarning.

error is timeout on trying to connect w/ /ip4/127.0.0.1/tcp/1
(node:5052) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: timeout on trying to connect w/ /ip4/127.0.0.1/tcp/1

Since _write() is working w/o promises by design I am pretty sure this warning is due to the code given here. But I can't figure out how to get rid of the warning. Even attaching catch-handler explicitly didn't suppress it.

UPDATE: Code has been refactored and re-tested. Rejection warning is logged nonetheless. I was using NodeJS 6.11 first, but this time it's 8.11.

what happens if you refactor your code to remove the promise constructor anti-pattern - i.e. `new Promise( ( writeResolve, writeReject ) => {` — Jaromanda X, Apr 17 '18 at 06:44
@JaromandaX I don't get your point. Do you want me to remove all promises from code? That would be no solution but a workaround. Due to using native support for Promises (no bluebird or similar) there is no opportunity to create Promises w/o that constructor-pattern. — Thomas Urban, Apr 17 '18 at 06:52
Try to [catch all unhandled promises](https://stackoverflow.com/questions/31472439/catch-all-unhandled-javascript-promise-rejections/41573379#41573379), it will tell you which line of code is throwing it. — laggingreflex, Apr 17 '18 at 06:52
No, I just thought you could simplify the code by removing the redundant promise constructor wrapping your code that deals with promises already - that's one of the problems with that anti-pattern - it can lead to things just like this — Jaromanda X, Apr 17 '18 at 06:53
@laggingreflex Thanks for the hint. I've injected that code resulting in two stack traces both starting at the line `setTimeout( reject, 1000, ... )`. So it eventually doesn't help as the code `timeout.catch( () => {} )` is catching errors on that initial promise. Just like `Promise.race()`, though it might fail to "handle" it in case of first promise being resolved first. That's why I consider that additional catch-handler as eligible. — Thomas Urban, Apr 17 '18 at 07:06
@Bergi Well, it isn't _my_ `doneFn`, but Node's. Nonetheless this seems to be related to the issue. If I don't pass **error** into `doneFn()`, but emit `error` event on socket beforehand, there is no warning. — Thomas Urban, Apr 17 '18 at 09:08

Lewis · Answer 1 · 2018-04-17T08:00:08.737

Frankly, your code is quite messy. You better rewrite the code like this before trying to debug the above issue.

_write( data, encoding, doneFn ) {
    Promise.race( [
        this.getSocket(),
        /* This below code should better be handled within method this.getSocket so we won't need to use Promise.race */
        new Promise((resolve, reject) => {
            setTimeout( reject, 1000, Object.assign( new Error( `timeout on trying to connect w/ ${this.address}` ), { code: "ECONNREFUSED" } ) );
        }),
    ] )
        .then( socket => {
            return socket && new Promise( ( resolve, reject ) => {
                socket.once( "error", reject );
                socket.write( data, encoding, () => {
                    socket.removeListener( "error", reject );

                    resolve();
                } );
            } );
        } )
        .then(doneFn)
        .catch(error => {
            const { address } = this;

            switch ( error.code ) {
                case "EPIPE" :
                case "ECONNRESET" :
                    Log( `lost connection w/ ${address.id}` );
                    break;

                case "ECONNREFUSED" :
                    Log( `failed to connect w/ ${address.id}` );
                    break;
            }
            doneFn( error );
        });
}

@cepharum Then try to remove Promise.race by checking the condition of 1000ms timeout within method `this.getSocket` instead. But this code still should work. Make sure that the error is not from somewhere else. — Lewis, Apr 17 '18 at 08:02
This is not an option due to changing the semantics of what is intentionally required there. Written packets should be dropped after some timeout, but trying to establish connection should not be stopped at the same time. And successive writes should use the same promise provided by getSocket() which is promising to have established connection to peer eventually. By moving timeout detection there this scenario wouldn't be possible anymore. Eventually, `Promise.race()` is there for a reason ... — Thomas Urban, Apr 17 '18 at 09:18

score 0 · Answer 2 · answered Apr 17 '18 at 07:50

Looks like stream implementations have issues w/ Promise.race().

I tried to break down the code to the essential pattern:

function someHandler( doneFn ) {
    const toBeResolvedLate = new Promise( ( resolve, reject ) => {
        setTimeout( resolve, 5000 );
    } );
    const toBeRejectedEarly = new Promise( ( resolve, reject ) => {
        setTimeout( reject, 1000, new Error( "timeout" ) );
    } );

    Promise.race( [
        toBeRejectedEarly,
        toBeResolvedLate,
    ] )
        .then( doneFn )
        .catch( error => {
            doneFn( error );
        } );
}


someHandler( ( ...args ) => {
    console.log( "directly:", args );
} );


const { Writable } = require( "stream" );

const stream = new Writable( {
    write: function( a, b, done ) {
        someHandler( ( ...args ) => {
            console.log( "in _write:", args );
            done( ...args );
        } );
    },
} );

stream.write( Buffer.from( "chunk" ) );

As a result I get this:

directly: [ Error: timeout
    at Promise (...\scratch_2.es6:6:30)
    at new Promise (<anonymous>)
    at someHandler (...\scratch_2.es6:5:29)
    at Object.<anonymous> (...\scratch_2.es6:20:2)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10) ]
in _write: [ Error: timeout
    at Promise (...\scratch_2.es6:6:30)
    at new Promise (<anonymous>)
    at someHandler (...\scratch_2.es6:5:29)
    at Writable.write [as _write] (...\scratch_2.es6:29:4)
    at doWrite (_stream_writable.js:397:12)
    at writeOrBuffer (_stream_writable.js:383:5)
    at Writable.write (_stream_writable.js:290:11)
    at Object.<anonymous> (...\scratch_2.es6:36:9)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10) ]
(node:3288) UnhandledPromiseRejectionWarning: Error: timeout
    at Promise (...\scratch_2.es6:6:30)
    at new Promise (<anonymous>)
    at someHandler (...\scratch_2.es6:5:29)
    at Writable.write [as _write] (...\scratch_2.es6:29:4)
    at doWrite (_stream_writable.js:397:12)
    at writeOrBuffer (_stream_writable.js:383:5)
    at Writable.write (_stream_writable.js:290:11)
    at Object.<anonymous> (...\scratch_2.es6:36:9)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
(node:3288) 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:3288) [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.

If I omit the last line of code trying to write chunk the second stack trace as well as the UnhandledPromiseRejectionWarning-stuff is gone.

Probably, don't call the `done` function of `write` with an error argument? — Bergi, Apr 17 '18 at 08:02
@Bergi Put it in an answer and I will make it the accepted one. — Thomas Urban, Apr 17 '18 at 09:09

Getting UnhandledPromiseRejectionWarning though catching rejections as good as possible

2 Answers2