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

mssql.connect generates error "socket must be an instance of net.Socket" / node:tls does not accept a custom stream for it's .socket param #5147

Open
dmcquay opened this issue Sep 12, 2023 · 21 comments
Labels
atw bug Something isn't working node.js Compatibility with Node.js APIs

Comments

@dmcquay
Copy link

dmcquay commented Sep 12, 2023

What version of Bun is running?

1.0.1+31aec4ebe325982fc0ef27498984b0ad9969162b

What platform is your computer?

Darwin 21.6.0 x86_64 i386

What steps can reproduce the bug?

bun init -y
bun add mssql

Create a file named index.js with the following contents:

import * as sql from 'mssql'

const main = async () => {
	await sql.connect({
		user: 'bogus',
		password: 'bogus',
		database: 'bogus',
		server: 'example.database.windows.net'
	})
}

main()

You can literally use that example server to reproduce. No need to set up your own SQL Server.

What is the expected behavior?

Connection error because the user/password is wrong:

ConnectionError: Cannot open server 'example' requested by the login. Client with IP address '136.38.114.139' is not allowed to access the server...

I am checking this by running the same file with node 20.5.0.

Of course in my real code, I can also connect to my server successfully, but I don't want to share my DB host publicly, so this was a more minimal reproduction I could share.

What do you see instead?

ConnectionError: Failed to connect to example.database.windows.net:1433 - socket must be an instance of net.Socket
code: "ESOCKET"

Additional information

I think the error is thrown here:

throw new TypeError("socket must be an instance of net.Socket");

But I haven't been able to figure out what line in mssql lib is triggering this.

@dmcquay dmcquay added the bug Something isn't working label Sep 12, 2023
@petterbergman
Copy link

Also reproduced on bun 1.0.2 with correct user name and password, thus making it impossible to use bun with mssql.

@darkme0w
Copy link

same issue :((

@jeremyd4500
Copy link

Same issue here connecting to Azure SQL Server with sequelize.

@andresverjan
Copy link

Same issue here connecting to Azure SQL Server with sequelize !!!

@ianzepp
Copy link

ianzepp commented Sep 29, 2023

Same error. Can connect to the Azure service just fine from the same local machine with Visual Code + the SQL server plugin. Firewall rules allow access. Bun + Knex fails to connect with the same config/env vars and throws a ESOCKET error with message socket must be an instance of net.Socket.

@cgslivre
Copy link

I had the same problem, it was not possible to connect to the SQL SERVER.

After updating to Bun 1.0.6, it apparently worked!

I'll do some more testing but it's possible that the team has already fixed the lack of net.Socket implementation.

Steps:

bun init . # confirm, confirm ...
bun add mssql
bun add knex

./index.ts

import knex from "knex";

const db = knex({
    client: "mssql",
    connection: {
        host: Bun.env["DB_DEV_HOST"],
        port: parseInt(Bun.env["DB_DEV_PORT"] || "1433"),
        user: Bun.env["DB_USER"],
        password: Bun.env["DB_PASS"],
        database: Bun.env["DB_DATABASE"],
    },
});

console.log(await db.table("example_table").first());

./package.json

{
    "name": "bot-bun",
    "module": "index.ts",
    "type": "module",
    "devDependencies": {
        "bun-types": "latest"
    },
    "peerDependencies": {
        "typescript": "^5.0.0"
    },
    "dependencies": {
        "knex": "^3.0.1",
        "mssql": "^10.0.1"
    }
}

Result:
print-20231014

@Electroid Electroid added the node.js Compatibility with Node.js APIs label Oct 27, 2023
@mscoobby
Copy link

It is still an issue with bun 1.0.11 Anyone found a fix?

@cgslivre
Copy link

cgslivre commented Nov 10, 2023

It is still an issue with bun 1.0.11 Anyone found a fix?

I tested it on 1.0.6 and it was OK.

I updated yesterday to 1.0.11, ran the test again, and it still seems OK!

image

@mscoobby
Copy link

That is very strange, I am using sequelize and I am still getting the error.

package.json

{
  "name": "bun-sequelize",
  "module": "index.ts",
  "type": "module",
  "devDependencies": {
    "bun-types": "latest"
  },
  "peerDependencies": {
    "typescript": "^5.0.0"
  },
  "dependencies": {
    "sequelize": "^6.35.0",
    "tedious": "^16.6.0"
  }
}

index.ts

import { Sequelize } from 'sequelize'

const sequelize = new Sequelize('', '--', '--', {
    port: 1450,
    dialect: 'mssql',
})

try {
    await sequelize.authenticate();
    console.log('Connection has been established successfully.');
} catch (error) {
    throw new Error(`Unable to connect to the database: ${error}`)
}

Result:
image

The database is running in Docker and its 1433 port is exposed on 1450
image

And the exact same file but executed with node:
image

@Angelelz
Copy link

Angelelz commented Dec 3, 2023

Same error in macOS sonoma. Everything works properly in node

@EthanTuning
Copy link

I am on Bun v1.0.20, and I am also getting this error when trying to connect to a MS SQL Server database using mssql. I also tried to connect with sequelize and same thing.

This seems to me to be a pretty big deal. Having no ability to connect to databases from a runtime that's suppose to run on servers, one of the most common things you would ever do on a server.

Is there possibly some other configuration needed to get this working?

@ReeceXW
Copy link

ReeceXW commented Jan 9, 2024

Can we get an update on this, how can bun be viable without even basic sql driver support?

@Electroid
Copy link
Contributor

It looks like the issue is that our version of tls.connect assumes that the socket must be a Socket, where indeed it is possible for it to be a Duplex.

socket <stream.Duplex> Establish secure connection on a given socket rather than creating a new socket. Typically, this is an instance of net.Socket, but any Duplex stream is allowed.

@paperdave paperdave added the atw label Jan 12, 2024
@paperdave
Copy link
Collaborator

dependency-free reproduction

import { connect } from 'node:tls';
import { Duplex } from 'node:stream';

let socket;
class DuplexSocket extends Duplex {
  constructor(options) {
    super(options);
  }
  _read() {
		console.log('pass');
		socket.destroy();
  }
  _write(chunk, encoding, callback) {
  }
  _final(callback) {
  }
}

socket = connect({
  host: "127.0.0.1",
  port: 1433,
  localAddress: undefined,
  family: 4,
	socket: new DuplexSocket()
});
const onError = err => {
	console.log('e', err);
};
const onConnect = () => {};
socket.on('error', onError);
socket.on('connect', onConnect);

@paperdave paperdave changed the title mssql.connect generates error "socket must be an instance of net.Socket" mssql.connect generates error "socket must be an instance of net.Socket" / node:tls does not accept a non-tls socket Jan 12, 2024
@paperdave paperdave changed the title mssql.connect generates error "socket must be an instance of net.Socket" / node:tls does not accept a non-tls socket mssql.connect generates error "socket must be an instance of net.Socket" / node:tls does not accept a custom stream for it's .socket param Jan 12, 2024
@paperdave
Copy link
Collaborator

image DuplexSocket is not - a socket - bun's socket

this is defined in the node.js documentation and we should implement it.
image

@Angelelz
Copy link

Angelelz commented Feb 3, 2024

We need to connect to a sqlserver database using node-mssql, it would be really good to be able to use bun at work. Please keep up the good work

@mvandervliet
Copy link

I recently came across this exact scenario and tried a number of things. Initially I tried just using mssql directly (without sequelize or knex), and got the same errors.

I suspected it might be a bun version thing, or a mssql version thing and tried multiple variations without success.

Then, upgrading to latest packages AND wrapping with knex solved for me:

  • bun: 1.0.26
  • mssql: 10.0.2
  • knex: 3.1.0

@JocularMarrow
Copy link

Same issue on our side,

  • bun: 1.0.26
  • mssql: 10.0.2

We have an existing project we wanna migrate to bun. But this issue makes it completely impossible, reworking our full code base to use knex or maybe try prisma will take to long.

Developers needs to look into this issue... This really makes bun in my eyes useless. Databases are somewhat important.

@SamMorey
Copy link

I am also getting blocked by this issue:

  • bun: 1.0.30
  • sequelize: 6.37.1
  • tedious: 17.0.0
  • knex: 3.1.0

Seconding @mvandervliet that Knex functions. We have an older project that we don't want to convert, so being able to use sequelize and tedious out of the box would be much nicer.

@yurisanp
Copy link

Same issue here

bun: 1.0.35
typeorm: 0.3.20
mssql: 10.0.2

I need a fix for this issue to continue migrating to Bun.

@anilcan-kara
Copy link

Still exists

Bun: 1.1.6
mssql: 10.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
atw bug Something isn't working node.js Compatibility with Node.js APIs
Projects
None yet
Development

No branches or pull requests