Align arguments of connect() in node:tls and node:net to Node (#10854)

[henrikstorck]

What does this PR do?

The handling of arguments in Bun's implementation of connect() (node:tls) deviates from the available combinations of arguments in Node. This breaks compatibility for many packages that rely on the order of parameters in Node.

I adjusted the connect method to allow the same amount and order of arguments as Node.

• Code changes

How did you verify your code works?

• I included a test for the new code, or existing tests cover it
• I ran my tests locally and they pass (bun-debug test test-file-name.test)

[github-actions]

❌ @henrikstorck, your commit has failing tests :(

💪 3 failing tests Darwin AARCH64

• test/cli/install/bun-install.test.ts 1 failing
• test/js/bun/http/serve.test.ts 1 failing
• test/js/node/http2/node-http2.test.js 1 failing

💻 4 failing tests Darwin x64 baseline

• test/cli/install/bunx.test.ts 1 failing
• test/cli/install/registry/bun-install-registry.test.ts 2 failing
• test/js/bun/http/serve.test.ts 1 failing
• test/js/web/workers/worker.test.ts 1 failing

💻 5 failing tests Darwin x64

• test/cli/install/bun-create.test.ts 2 failing
• test/cli/install/registry/bun-install-registry.test.ts 2 failing
• test/js/bun/http/serve.test.ts 2 failing
• test/js/bun/spawn/spawn-kill-signal.test.ts 1 failing
• test/js/bun/test/test-test.test.ts 1 failing

🪟💻 8 failing tests Windows x64 baseline

• test/cli/install/bunx.test.ts 1 failing
• test/cli/test/bun-test.test.ts 1 failing
• test/integration/next-pages/test/dev-server-ssr-100.test.ts 1 failing
• test/integration/next-pages/test/dev-server.test.ts 1 failing
• test/integration/next-pages/test/next-build.test.ts 1 failing
• test/js/bun/shell/bunshell.test.ts 1 failing
• test/js/bun/spawn/spawn.test.ts 1 failing
• test/js/node/dns/node-dns.test.js 2 failing

🪟💻 10 failing tests Windows x64

• test/cli/install/bunx.test.ts 1 failing
• test/cli/install/registry/bun-install-registry.test.ts 2 failing
• test/integration/esbuild/esbuild.test.ts 1 failing
• test/integration/next-pages/test/dev-server-ssr-100.test.ts 1 failing
• test/integration/next-pages/test/dev-server.test.ts 1 failing
• test/integration/next-pages/test/next-build.test.ts 1 failing
• test/js/bun/shell/bunshell.test.ts 1 failing
• test/js/bun/shell/commands/rm.test.ts 1 failing
• test/js/bun/spawn/spawn.test.ts 1 failing
• test/js/node/dns/node-dns.test.js 2 failing

View logs

[henrikstorck]

Might fix some of the following issues:
#10711
#7851
#6681
#4540
#4136
#4033
#3701
#3170
taskforcesh/bullmq#2237

Maybe related to this but unlikely:
#5147

[Jarred-Sumner]

Thank you for this. Looks like there are some failing node:tls tests. Our tests might be wrong and need to be updated.

[henrikstorck]

Thank you for this. Looks like there are some failing node:tls tests. Our tests might be wrong and need to be updated.

Do you mean the github-actions message from 20h ago?

I pushed a few commits since then. Should be a lot less failing tests now

[henrikstorck]

@Jarred-Sumner It just finished re-running the tests and no more failing node:tls tests I think

[nektro]

this appears to have broken test/js/third_party/postgres/postgres.test.ts on all platforms and is now showing up in other PRs. reverting the commit locally fixes it. the failure did not show up here because tests were skipped since this came from a contribution and the TLS_POSTGRES_DATABASE_URL was thus not exposed to the CI environment.

[nektro]

Going to revert this and revisit. Apologies!

[henrikstorck]

@nektro Sorry!
Thank you for noticing though.

Are there any other tests failing because of this?
Would like to make sure they all pass again before recreating the PR

[nektro]

no worries! thank u for the patch :)

that looked to be like the only one. all the new ones you added worked. I'm working on making a new PR to test all the changes and will add you as co-author

[henrikstorck]

Hey @nektro, were you able to reproduce the failing tests locally?
When I use my local db and run the test, they pass.
Not sure what I'm doing wrong.
Thanks in advance.

[nektro]

yeah with 6217d78 checked out and making a fresh build the Postgres test file was timing out for me locally