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

TLSSocket._start seems never set #18303

Closed
dsherret opened this issue Nov 17, 2022 · 20 comments · Fixed by #20120
Closed

TLSSocket._start seems never set #18303

dsherret opened this issue Nov 17, 2022 · 20 comments · Fixed by #20120
Assignees
Labels
bug Something isn't working correctly node compat

Comments

@dsherret
Copy link
Member

dsherret commented Nov 17, 2022

Describe the bug

https://github.com/denoland/deno_std/blob/bc345306e520e758e8af9b00144c393c9c066c52/node/_tls_wrap.ts#L373

Steps to Reproduce

import sql from "npm:mssql";

const connectionPool = new sql.ConnectionPool({
  server: "localhost",
  database: "SomeDatabase",
  user: "user",
  password: "password",
  options: {
    trustedConnection: true,
    trustServerCertificate: true,
  },
});

const connection = await connectionPool.connect();
console.log("Connected");

Expected behavior

Should work.

Environment

  • OS: Windows 10
  • deno version: 1.28.1
  • std version: 1.165.0
@superted17
Copy link

If it helps, here is another scenario in which this issue can also be seen:

import Imap from 'https://esm.sh/[email protected]';

const imapConfig: Imap.Config = {
  user: 'redacted',
  password: 'redacted',
  host: 'redacted',
  port: 993,
  tls: true
};

const imap = new Imap(imapConfig);

imap.once('ready', () => {
  console.info('ready');
});

imap.once('error', (err: Error) => {
  console.error(err);
});
 
imap.once('end', () => {
  console.info('Connection ended');
});

imap.connect();

Error stack trace:

❯ deno run --allow-env index.ts                                                                                                                                                           ─╯
error: Uncaught TypeError: tlssock._start is not a function
    tlssock._start();
            ^
    at Object.connect (https://deno.land/[email protected]/node/_tls_wrap.ts:365:13)
    at d.connect (https://esm.sh/v96/[email protected]/deno/imap.js:3:1490)
    at file:///home/ed/code/incoming-email/index.ts:27:6

@laidybug
Copy link

laidybug commented Dec 2, 2022

Can anyone help with this? From what I see, the _start function of the TLSSocket class in the file _tls_wrap.ts is missing. I am very willing to help work on it, but I don't know what the _start function should do.

It seems like the TLS module for Node Compat flat out doesn't work because of this, and it doesn't seem straightforward to update a typical Node library to use Deno's primary TLS support instead of the Node Compat support due to major differences between the APIs.

I would appreciate any help on this, and would be willing to write code for it if someone can provide direction (e.g., what the _start function is supposed to do).

@laidybug
Copy link

laidybug commented Dec 2, 2022

I found the relevant Node code: https://github.com/nodejs/node/blob/main/lib/_tls_wrap.js#L952

TLSSocket.prototype._start = function() {
  debug('%s _start',
        this._tlsOptions.isServer ? 'server' : 'client',
        'handle?', !!this._handle,
        'connecting?', this.connecting,
        'requestOCSP?', !!this._tlsOptions.requestOCSP,
  );
  if (this.connecting) {
    this.once('connect', this._start);
    return;
  }

  // Socket was destroyed before the connection was established
  if (!this._handle)
    return;

  if (this._tlsOptions.requestOCSP)
    this._handle.requestOCSP();
  this._handle.start();
};

It calls this._handle.start() which doesn't seem to be implemented yet. It looks like this._handle is a Socket but it doesn't have a start method. Currently, the Node Compat version (_tls_wrap.ts) is less than 1/3 the number of lines as the Node version (_tls_wrap.js), so I'm not sure how close it is to functioning in Deno, but I'd very much like to help get it working and will continue digging into it.

@kt3k
Copy link
Member

kt3k commented Dec 22, 2022

npm:nodemailer seems having the same problem denoland/std#3035

@shivajivarma
Copy link

Any update on this?

@dehrhard
Copy link

Waiting for this missing puzzle piece to use nodemailer 🥧

@coreybutler
Copy link

This also impacts ldapjs when using an ldaps:// host.

@bartlomieju
Copy link
Member

Hey folks, were you able to try the fix available in Deno v1.36.1 for all the packages listed in the issue? I would appreciate some feedback here to know if you encounter any more problems.

@Niki2k1
Copy link

Niki2k1 commented Aug 24, 2023

the mssql npm library and nodemailer are working now🥳

@alitavakoli96
Copy link

alitavakoli96 commented Aug 25, 2023

Hey folks, I'm getting socket hangup error when connecting to azure SQL db I used to get the same error before, but when I upgraded to 1.36.3 from 1.36.1 the issue happening here got fixed but now I'm getting socket hangup error.

Edit: I tested my connection setting with mssql in pure node js (made a tiny script just to see if I'm tripping with my connection setting). Well the pure node js works fine, no issues, could query a table too. same setting, same latest mssql version (9.1.3 ).

then I went and tested the code using deno in isolation, again same setting, same latest mssql version (9.1.3) and I get the same error as my original project. So its definetly something to do with the latest changes in deno.

This is the stacktrace:

at file:///.../deno/npm/registry.npmjs.org/mssql/9.1.3/lib/tedious/connection-pool.js:70:17 at Connection.onConnect (file:///.../deno/npm/registry.npmjs.org/tedious/15.1.3/lib/connection.js:1012:9) at Object.onceWrapper (ext:deno_node/_events.mjs:507:26) at Connection.emit (ext:deno_node/_events.mjs:382:28) at Connection.emit (file:///.../deno/npm/registry.npmjs.org/tedious/15.1.3/lib/connection.js:1040:18) at Connection.socketError (file:///.../deno/npm/registry.npmjs.org/tedious/15.1.3/lib/connection.js:1395:12) at Connection.socketEnd (file:///...l/deno/npm/registry.npmjs.org/tedious/15.1.3/lib/connection.js:1415:12) at Socket.<anonymous> (file:///.../deno/npm/registry.npmjs.org/tedious/15.1.3/lib/connection.js:1155:16) at Socket.emit (ext:deno_node/_stream.mjs:1857:11) at endReadableNT (ext:deno_node/_stream.mjs:3646:16)

@dehrhard
Copy link

Hey hey, unfortunately I'm getting a similar error for nodemailer:

 error: Uncaught TypeError: this._socket.removeListener is not a function
     at SMTPConnection._upgradeConnection (file:///home/deno/server/node_modules/.deno/[email protected]/node_modules/nodemailer/lib/smtp-connection/index.js:897:22)
     at file:///home/deno/server/node_modules/.deno/[email protected]/node_modules/nodemailer/lib/smtp-connection/index.js:247:29
     at Object.action (ext:deno_web/02_timers.js:153:11)
     at handleTimerMacrotask (ext:deno_web/02_timers.js:67:10)
     at eventLoopTick (ext:core/01_core.js:189:21)

The offending parts in the code I marked here
image
Curiously this._socket.removeListener is called before tls.connect so this must have worked before. I don't understand.

Now I haven't worked on this project for a while so I can't exclude user error but it seems pretty deep in the core code.

@bartlomieju
Copy link
Member

Thanks for the reports; we'll look into that.

@bartlomieju
Copy link
Member

@dehrhard could you share the reproduction code for the issue you are encountering?

@dehrhard
Copy link

@bartlomieju It's pretty much as simple as it gets. Probably it's something stupid.

// @deno-types="npm:@types/nodemailer"
import { createTransport, SendMailOptions } from "npm:nodemailer";

const connection = {
  from: "[email protected]",
  host: "host.host.com",
  port: 587,
  auth: {
    user: "123",
    pass: "456",
  },
};

const smtpClient = createTransport({
  connection,
  secure: true,
});

const options: SendMailOptions = {
  from: "[email protected]",
  to: "[email protected]",
  subject: "Test",
  text: "Content",
  html: "<h1>HTML</h1>",
};
smtpClient.sendMail(options);

deno run -A main.ts
error: Uncaught TypeError: this._socket.removeListener is not a function
    at SMTPConnection._upgradeConnection (file:///home/x/.cache/deno/npm/registry.npmjs.org/nodemailer/6.8.0/lib/smtp-connection/index.js:878:22)
    at file:///home/x/.cache/deno/npm/registry.npmjs.org/nodemailer/6.8.0/lib/smtp-connection/index.js:246:26
    at Object.action (ext:deno_web/02_timers.js:153:11)
    at handleTimerMacrotask (ext:deno_web/02_timers.js:67:10)
    at eventLoopTick (ext:core/01_core.js:189:21)
deno --version
deno 1.36.3 (release, x86_64-unknown-linux-gnu)
v8 11.6.189.12
typescript 5.1.6

@shivajivarma
Copy link

shivajivarma commented Sep 21, 2023

If it helps, here is another scenario in which this issue can also be seen:

import Imap from 'https://esm.sh/[email protected]';

const imapConfig: Imap.Config = {
  user: 'redacted',
  password: 'redacted',
  host: 'redacted',
  port: 993,
  tls: true
};

const imap = new Imap(imapConfig);

imap.once('ready', () => {
  console.info('ready');
});

imap.once('error', (err: Error) => {
  console.error(err);
});
 
imap.once('end', () => {
  console.info('Connection ended');
});

imap.connect();

Error stack trace:

❯ deno run --allow-env index.ts                                                                                                                                                           ─╯
error: Uncaught TypeError: tlssock._start is not a function
    tlssock._start();
            ^
    at Object.connect (https://deno.land/[email protected]/node/_tls_wrap.ts:365:13)
    at d.connect (https://esm.sh/v96/[email protected]/deno/imap.js:3:1490)
    at file:///home/ed/code/incoming-email/index.ts:27:6

This still does not works. It is throwing different error now.

@sant123
Copy link

sant123 commented Sep 21, 2023

@shivajivarma what error? Could you post it please?

@shivajivarma
Copy link

shivajivarma commented Sep 21, 2023

@shivajivarma what error? Could you post it please?

deno 1.37.0

error: Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'rid')
    at TCP.#read (ext:deno_node/internal_binding/stream_wrap.ts:226:46)
    at TCP.readStart (ext:deno_node/internal_binding/stream_wrap.ts:96:17)
    at _tryReadStart (node:net:276:30)
    at TLSSocket._read (node:net:856:7)
    at TLSSocket.Readable.read (ext:deno_node/_stream.mjs:2997:16)
    at TLSSocket.read (node:net:789:34)
    at nReadingNextTick (ext:deno_node/_stream.mjs:3326:13)
    at Array.processTicksAndRejections (ext:deno_node/_next_tick.ts:30:15)
    at eventLoopTick (ext:core/01_core.js:180:29)

@sant123
Copy link

sant123 commented Sep 21, 2023

Perhaps related? #20594

@vectorkovacspeter
Copy link

the mssql npm library and nodemailer are working now🥳

Do you have sample for this? I still got the same error as the issue creator with latest deno and latest npm sqlserver package 🤔

@pikanezi
Copy link

pikanezi commented Feb 1, 2024

Still have this issue while using the imap npm library when setting tls to true with deno 1.40.2:

error: Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'Symbol(Deno.internal.rid)')
    at TCP.#read (ext:deno_node/internal_binding/stream_wrap.ts:220:45)
    at TCP.readStart (ext:deno_node/internal_binding/stream_wrap.ts:90:17)
    at _tryReadStart (node:net:276:30)
    at TLSSocket._read (node:net:856:7)
    at TLSSocket.Readable.read (ext:deno_node/_stream.mjs:2999:16)
    at TLSSocket.read (node:net:789:34)
    at nReadingNextTick (ext:deno_node/_stream.mjs:3328:13)
    at processTicksAndRejections (ext:deno_node/_next_tick.ts:30:15)
    at runNextTicks (ext:deno_node/_next_tick.ts:71:3)
    at eventLoopTick (ext:core/01_core.js:70:21)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet