Initialized receive timeout and changed exception to warning #118
Conversation
There was a problem hiding this comment.
It definitively makes sense to initialize the timeout by default.
Concerning the exception vs. warning output: It probably is a matter of control flow definition. The aim was that if the read fails, we clearly communicate that to the caller. By simply returning an empty string the caller has to explicitly check whether the returned string does make sense. A simple check against an empty string might be misleading, as this would require that a dashboard call would never return an empty string (which might be true, actually). By throwing an exception a caller can make a distinct decision what to do in that case by defining an exit strategy inside a try/catch block.
Just for the sake of clarity: I'm not saying that this is the one and only way to handle this, but this is a workflow we've used at multiple parts of this library, so far.
Codecov ReportBase: 41.13% // Head: 51.15% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #118 +/- ##
===========================================
+ Coverage 41.13% 51.15% +10.02%
===========================================
Files 76 82 +6
Lines 2064 2465 +401
Branches 256 279 +23
===========================================
+ Hits 849 1261 +412
+ Misses 1102 1072 -30
- Partials 113 132 +19
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This will fix issue #115