Add optional callback ranch_transport:format_error/1
#354
Conversation
| @@ -106,9 +106,14 @@ hide_socket_opts([{password, _}|SocketOpts]) -> | |||
| hide_socket_opts([SocketOpt|SocketOpts]) -> | |||
| [SocketOpt|hide_socket_opts(SocketOpts)]. | |||
|
|
|||
| format_error(no_cert) -> | |||
| format_error(_, no_cert) -> | |||
There was a problem hiding this comment.
This actually belongs in ranch_ssl:format_error/1 (and is replicated there). For backwards compatibility with existing custom transports without an format_error implementation which may rely on ranch_acceptors_sup handling this, I left it in.
src/ranch_acceptors_sup.erl
Outdated
| true -> | ||
| Transport:format_error(Reason); | ||
| false -> | ||
| io_lib:format("~999999p", [Reason]) |
There was a problem hiding this comment.
Not sure if we can do better?
There was a problem hiding this comment.
Ah, yeah... We should change that in other places as well then. In this PR, or a separate one?
There was a problem hiding this comment.
In this PR is fine but a separate commit.
| "no certificate provided; see cert, certfile, sni_fun or sni_hosts options"; | ||
| format_error(reuseport_local) -> | ||
| format_error(_, reuseport_local) -> |
There was a problem hiding this comment.
This is generated by ranch_acceptors_sup itself as it is a misconfiguration specific to ranch. Therefore, I left it here, it doesn't seem to fit into a transport.
| format_error(Reason) -> | ||
| inet:format_error(Reason). | ||
| format_error(Transport, Reason) -> | ||
| case erlang:function_exported(Transport, format_error, 1) of |
There was a problem hiding this comment.
Add a comment saying this will be required in 3.0, and for now let's consider/document it as optional.
|
Looks fine yes. |
|
Maybe have a test for the custom transport if we don't already. |
I think we don't. There is |
|
Wait, we have concuerror tests? 😳 Since when? That completely went by me 😅 |
|
Yes. #261 |
|
It's very basic for now: https://github.com/ninenines/ranch/blob/master/test/ranch_concuerror.erl |
juhlig
force-pushed
the
ranch_transport_format_error
branch
from
5de33d5
to
90da425
Compare
|
@essen added docs and a test and changed |
Fixes #346.
No docs yet, wanted to see what you think of the implementation first. Will add more comments inline.