celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
17 stars 16 forks source link

log: Change `stream reset`-related errors from ERROR to WARN level #140

Closed renaynay closed 3 months ago

renaynay commented 5 months ago

Throughout the lib, we use ERROR level logs for stream resets. We should use WARN instead to not cause node operators to think there is something wrong as stream resets can occur for a variety of reasons.

Related https://github.com/celestiaorg/celestia-node/issues/3063

triggeredcode commented 5 months ago

@renaynay Can I help here?

renaynay commented 5 months ago

Sure! @triggeredcode

triggeredcode commented 5 months ago

Hi @renaynay, Could you provide more details on differentiating stream resets from other errors? Any additional context would be helpful. Thanks!

AryanGodara commented 3 months ago

@renaynay Does this issue also close the corresponding issue on celestia-node? Because there seems to be no response on that issue as well 🤔

Wondertan commented 3 months ago

@triggeredcode, to find places where stream reset may happen you need to look up all the places where streams are used in exchange implementation. Particularly we ate interested in write and read operations

Wondertan commented 3 months ago

@AryanGodara, yed, this would close the corresponding issue

AryanGodara commented 3 months ago

@triggeredcode, to find places where stream reset may happen you need to look up all the places where streams are used in exchange implementation. Particularly we ate interested in write and read operations

Can I please work on this issue then? There seems to be no activity on it for some time now. I'll try to get a PR ready in a day or two :D

Wondertan commented 3 months ago

@AryanGodara, sure, go for it!

AryanGodara commented 3 months ago

@Wondertan Can you please assign this to me so there are no conflicting contribs 😅