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

Added error to write #104

Merged
merged 1 commit into from
Feb 26, 2021
Merged

Added error to write #104

merged 1 commit into from
Feb 26, 2021

Conversation

Tobbeman
Copy link
Contributor

This error can be used to detect if the connection was broken,
for example if the server goes down between client sending data.

psuedo code:
reply, err := s.exec()
if err != nil {
if err.Error() == netconf.BrokenConnectionError {
retry()
}
}

In my current use case I need to know if the server goes down, since the client will be living a long time, connection between server and client can at some point go down. This fix will let me detect that.

Copy link
Collaborator

@nemith nemith left a comment

Choose a reason for hiding this comment

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

Yes this should be error checked. Good catch.

netconf/transport.go Outdated Show resolved Hide resolved
netconf/transport.go Outdated Show resolved Hide resolved
@Tobbeman Tobbeman requested a review from nemith January 26, 2021 07:29
netconf/transport.go Outdated Show resolved Hide resolved
netconf/transport.go Outdated Show resolved Hide resolved
@Tobbeman
Copy link
Contributor Author

@nemith
Can you take another look?

netconf/transport.go Outdated Show resolved Hide resolved
This error can be used to detect if the connection was broken,
for example if the server goes down between client sending data
@nemith
Copy link
Collaborator

nemith commented Feb 26, 2021

Thanks for fixing this and sorry for the long review time. I guess no one else is really maintaining this package.

@nemith nemith merged commit bb51e84 into Juniper:master Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants