sidorares / sidorares/node-mysql2

There's no possibility to attach handlers to connection before it's opened

Open
#916 3 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
4.4k
Forks
680
Avg merge
9h 7m
Merged PRs (30d)
59

Description

Hello!

Test scenario is:

  1. make pool with the following configuration:
{
  connectionLimit: 10,
  connectTimeout: 1000
}
  1. create script which acquires connection from the pool and executes smth against the DB with 1 second interval
  2. after 3 seconds of running the script, cut the wire

So at the second nr 6, intervalled script will still try to open connections, as connectionLimit isn't reached yet, so pool tries to open new connection (https://github.com/sidorares/node-mysql2/blob/821b2bcdc254c483e6879b5d4afc9757c55c7ed7/lib/pool.js#L50).

while pool will get this connection only once connection is established:
https://github.com/sidorares/node-mysql2/blob/821b2bcdc254c483e6879b5d4afc9757c55c7ed7/lib/pool.js#L61

which means, that if the connectTimeout is hit, here:
https://github.com/sidorares/node-mysql2/blob/master/lib/connection.js#L120

emitted error from the connection will cause the node process to die.

proposed solution would be to move this:
https://github.com/sidorares/node-mysql2/blob/master/lib/pool.js#L61

right after connection is constructed:
https://github.com/sidorares/node-mysql2/blob/master/lib/pool.js#L53

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/pool.js at connection construction and handler registration, then inspect the timeout path in lib/connection.js. Reproduce the interrupted-wire scenario and verify that a connection timing out before pool acquisition no longer causes an uncaught process error.

Written by the indexing model from the issue text.

Assessment

Tech stack
mysql, node.js
Domain
backend, databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.