Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to catch error during tls.connect(duplex) #29678

Open
Qualtagh opened this issue Sep 23, 2019 · 2 comments
Open

Impossible to catch error during tls.connect(duplex) #29678

Qualtagh opened this issue Sep 23, 2019 · 2 comments
Labels
tls Issues and PRs related to the tls subsystem.

Comments

@Qualtagh
Copy link

  • Version: 10.16.3
  • Platform: Ubuntu 16.04.1
  • Subsystem: tls

There seems to be no way to catch synchronous error of underlying duplex stream during tls.connect operation.
Code sample:

const stream = require('stream');
const tls = require('tls');

const async = false;
process.on('uncaughtException', e=>console.log('uncaught: '+e));

const socket = new stream.Duplex({
    read(size){},
    write(data, encoding, cb){
        let error = new Error('intended error');
        if (async)
            setTimeout(()=>cb(error), 1000);
        else
            cb(error);
    },
});
socket.on('error', e=>console.log('socket error: '+e));

const tls_socket = tls.connect({socket});
tls_socket.on('error', e=>console.log('tls_socket error: '+e));

Expected output:

socket error: Error: intended error
tls_socket error: Error: intended error

Actual output:

socket error: Error: intended error
uncaught: Error: intended error

Changing async to true solves the issue.
Can we allow passing onError handler to tls.connect method? So, it will be set before calling _start() method of TLSSocket.

@Qualtagh Qualtagh changed the title Can't catch error during tls.connect(duplex) Impossible to catch error during tls.connect(duplex) Sep 26, 2019
@lpinca lpinca added the tls Issues and PRs related to the tls subsystem. label Sep 28, 2019
@cjihrig
Copy link
Contributor

cjihrig commented Nov 27, 2019

FWIW, Node 13 yields the expected output.

@Qualtagh
Copy link
Author

Thank you! Yes, it's fixed now. Seems like error is emitted in nextTick in 13.
For 10, we worked around via copying contents of tls.connect and passing error listener in the beginning.
So, since it has workaround for old releases and fixed in latest release, perhaps it's ok to close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants