trigger-http: Log TLS startup errors instead of propagating #2230
Conversation
Signed-off-by: Lann Martin <lann.martin@fermyon.com>
| @@ -344,8 +344,10 @@ impl HttpTrigger { | |||
|
|
|||
| loop { | |||
| let (stream, addr) = listener.accept().await?; | |||
| let stream = acceptor.accept(stream).await?; | |||
| Self::serve_connection(self_.clone(), stream, addr); | |||
| match acceptor.accept(stream).await { | |||
There was a problem hiding this comment.
If acceptor.accept is a bad state where it will always error this might just turn into an infinite loop no? Should we log and return the error?
There was a problem hiding this comment.
I don't think that should be an issue with the TLS acceptor. Note that the TCP listener just above does still bail.
There was a problem hiding this comment.
I don't know enough about the TLS acceptor to say whether this is fine to do or not. I can't really give too much more of opinion without knowing why we don't want to propagate errors.
There was a problem hiding this comment.
The TLS acceptor handles the TLS handshake. It could fail because of an underlying TCP error but it can also fail because the handshake cannot be completed (for any number of reasons, some of which aren't under the server's control). Any error that is permanently fatal to the TCP listener should fail on the next loop iteration's call to listener.accept.
This prevents TLS handshake errors from exiting the server loop.