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

ConnectionMonitor: abortConnectionOnPingFailure is not used #2678

Closed
mkermani144 opened this issue Sep 4, 2024 · 2 comments
Closed

ConnectionMonitor: abortConnectionOnPingFailure is not used #2678

mkermani144 opened this issue Sep 4, 2024 · 2 comments
Labels
effort/hours Estimated to take one or several hours exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue

Comments

@mkermani144
Copy link

  • Version: 1.9.2
  • Platform: Darwin / Linux
  • Subsystem: Connection Monitor

Severity: Medium

Description:

There is a flag in connection manager init that is used nowhere:

/**
* If true, any connection that fails the ping will be aborted
*
* @default true
*/
abortConnectionOnPingFailure?: boolean

While it should probably be used here:

.catch(err => {
this.log.error('error during heartbeat, aborting connection', err)
conn.abort(err)
})

Steps to reproduce the error:

The flag is not used in the codebase other than the interface whose link is sent above. A global search will suffice.

@mkermani144 mkermani144 added the need/triage Needs initial labeling and prioritization label Sep 4, 2024
@achingbrain
Copy link
Member

Looks like using that option as a gate before aborting the connection was missed. Would you like to open a PR that implements the feature?

@achingbrain achingbrain added help wanted Seeking public contribution on this issue exp/beginner Can be confidently tackled by newcomers effort/hours Estimated to take one or several hours good first issue Good issue for new contributors and removed need/triage Needs initial labeling and prioritization labels Sep 4, 2024
@mkermani144
Copy link
Author

Seems I'm late to the party.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/beginner Can be confidently tackled by newcomers good first issue Good issue for new contributors help wanted Seeking public contribution on this issue
Projects
None yet
Development

No branches or pull requests

2 participants