fix(promise): forward pool error events - #4487
GiHoon1123 wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4487 +/- ##
=======================================
Coverage 92.50% 92.51%
=======================================
Files 93 93
Lines 15909 15915 +6
Branches 2275 2275
=======================================
+ Hits 14717 14723 +6
Misses 1192 1192
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@wellwelwel don't merge just yet. Wanted to check first that pool tracks idle connection errors at all - that might be not the case |
|
Traced it — you're right to hold off.
this.once('error', () => {
this._removeFromPool();
});
Making Scoping this PR back to what #3749 actually asked for: the type is missing for an event |
|
Correction on my last comment: |
Pool never emits 'error' itself - PoolConnection swallows connection
errors internally (once('error', () => this._removeFromPool())), so
the forwarding added here had nothing to forward. Scoping this back
to what sidorares#3749 actually needs.
|
Closing this. Once the error-forwarding piece is dropped there's nothing left to add here - the original request was for a type on an event Pool doesn't actually emit. If the idle-connection-error gap is worth fixing, that's a separate, bigger change to PoolConnection's own error handling, not this PR. |
Refs #3749
Promise pools inherit lifecycle events from the underlying core pool, but the error event was not forwarded and was not exposed in the promise pool types.
This adds error to the forwarded pool events and adds a matching
pool.on('error', ...)overload for the promise API. The existing promise wrapper event test now covers error forwarding, and the TypeScript build tests cover the promise pool event signature.Tests:
npm run typechecknpm run lintnpm run test:build