Skip to content

Conversation

@HappyZombies
Copy link
Contributor

This PR intends to address #3764 by introducing a state property on the connection object, aligning behavior with the original mysql package.

The new property reflects the current connection status; it only includes the following three states: disconnected, connected, or protocol_error.

The state flow includes:

Default: 'disconnected'

Updated to 'connected' upon successful handshake.

Updated to 'disconnected' after .end() or .close() is called (note that failures can eventually call this method).

Updated to 'protocol_error' on fatal or unexpected protocol errors.

Tests include:

Connects successfully and sets state to connected.

Closes cleanly and sets state to disconnected.

Incorrect credentials result in state protocol_error.

Let me know what you think, and tell me if any changes should be made -- along if we need more tests as well :)

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.51%. Comparing base (52bb6e4) to head (bcb70ab).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
lib/base/connection.js 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3885      +/-   ##
==========================================
+ Coverage   89.78%   90.51%   +0.73%     
==========================================
  Files          86       86              
  Lines       13607    13614       +7     
  Branches     1607     1635      +28     
==========================================
+ Hits        12217    12323     +106     
+ Misses       1390     1291      -99     
Flag Coverage Δ
compression-0 89.62% <88.88%> (+0.73%) ⬆️
compression-1 90.49% <88.88%> (+0.73%) ⬆️
static-parser-0 88.09% <88.88%> (+0.73%) ⬆️
static-parser-1 88.85% <88.88%> (+0.73%) ⬆️
tls-0 89.89% <77.77%> (+0.70%) ⬆️
tls-1 90.17% <88.88%> (+0.61%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@wellwelwel wellwelwel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A tip would be to use the sleep method instead of setting the setTimeout multiple times with a Promise, for example:

import { sleep } from 'poku';

// ...
await sleep(20);
// ...

Comment on lines +82 to +89
const raw = connection.connection;
// use changeUser to induce auth error (assuming user 'nope' doesn't exist)
let gotErr = null;
assert.equal(raw.state, "connected");
raw.on('error', (e) => { gotErr = e; });
try {
await connection.changeUser({ user: 'nope', password: 'bad' });
} catch (err) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const raw = connection.connection;
// use changeUser to induce auth error (assuming user 'nope' doesn't exist)
let gotErr = null;
assert.equal(raw.state, "connected");
raw.on('error', (e) => { gotErr = e; });
try {
await connection.changeUser({ user: 'nope', password: 'bad' });
} catch (err) {
const raw = connection.connection;
let gotErr = null;
assert.equal(raw.state, "connected");
raw.on('error', (e) => { gotErr = e; });
try {
await connection.changeUser({ user: 'NON_EXISTENT_USER', password: 'BAD_PASSWORD' });
} catch (err) {

assert.equal(badConnection.connection.state, 'protocol_error');
} finally {
try {
await bad.end();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
await bad.end();
await badConnection.end();

});

await it('state is protocol_error if creds are wrong.', async () => {
const badConnection = common.createConnection({ password: "WR0NG", user: 'wrong' }).promise();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const badConnection = common.createConnection({ password: "WR0NG", user: 'wrong' }).promise();
const badConnection = common.createConnection({ password: "WR0NG_PASSWORD", user: 'WRONG_USER' }).promise();

@wellwelwel wellwelwel added mysqljs/mysql incompatibilities Previously: feligxe-mysql-incompatibilities and removed needs documentation labels Oct 24, 2025
@HappyZombies
Copy link
Contributor Author

I just realized I forgot to add the "authorized" state, so I will be adding that and also fixing the unit tests as per your recommendation :)

@wellwelwel wellwelwel linked an issue Nov 27, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mysqljs/mysql incompatibilities Previously: feligxe-mysql-incompatibilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request: Public API for connection validation

2 participants