search

nemith / netconf

NETCONF implementation in Go.
Other
29 stars 7 forks source link

bug fix in main receive loop #85

Open ksproska opened 6 days ago

ksproska commented 6 days ago

before fix goroutine did not finish after TCP connection was closed

Fixes: #84

nemith commented 6 days ago

Thanks for the contribution!

I am not sure this is exactly the direction we want to go in. I think the current check was a bit of a hack that i put in to get the goroutine to close so I am not surprised it proved to a bit unreliable.

That being said I am not a fan of ever doing comparisons with the error message. Also two of the errors are http/http2 which so far there aren't any supported NETCONF transports that use http or http2. I do know there is an RFC to support NETCONF over QUIC/HTTP3 but these errors wouldn't catch that.

Is Nokia doing NETCONF over HTTP or was a mistake (or an overeager LLM?)

ksproska commented 3 days ago

I understand the reluctance to check error message content, I've read your thoughts and uploaded a new solution. It is similar to the second approach you've suggested with this difference that instead of broad net.Error interface the error is checked for specific type net.OpErr since it carries the error that causes trouble (screen from our program debug for context):

image

What do you think about this solution?