rrwright commented on code in PR #1205:
URL: https://github.com/apache/pekko-connectors/pull/1205#discussion_r2365855267
##########
sse/src/main/scala/org/apache/pekko/stream/connectors/sse/scaladsl/EventSource.scala:
##########
@@ -111,7 +113,12 @@ object EventSource {
.flatMap(Unmarshal(_).to[EventSource])
.fallbackTo(Future.successful(noEvents))
}
- def recover(eventSource: EventSource) =
eventSource.recoverWithRetries(1, { case _ => noEvents })
+ def recover(eventSource: EventSource) = eventSource.recoverWithRetries(1,
+ {
+ case e =>
+ log.warning(e, "SSE Connector is retrying failed stream for: {} ",
uri)
Review Comment:
yeah, I had similar iffy feelings on whether a warning is appropriate. But
info level definitely seems too light to express a genuine problem. It is a
connection error. But the default behavior is to reconnect forever.
Technically, I think this is roughly equivalent to a "Connection Reset" error,
which usually is (forcibly) viewable by the end user. But the code for infinite
retries basically makes the choice for the user. The best code design would
probably include some user-level choice about whether to silently retry forever
or be notified. I tried to stay away from editing the code logic… otherwise I'd
want to rewrite most of it. :-)
Maybe this would be best: make it log at the info level and add
documentation at least stating that it will retry forever, and connection error
details will only be available at the info level. Although there is an
exception at this point in the code and the `info` level doesn't accept logging
a `cause: Throwable`. We could `toString` it any of several different ways. I
picked a way and updated the code.
Additional opinions on what log level or exception string format is
appropriate would be appreciated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
