On Thu, Apr 4, 2019 at 9:09 AM Oleg Kalnichevski <[email protected]> wrote:
> On Thu, 2019-04-04 at 08:52 -0400, Gary Gregory wrote:
> > Hi All:
> >
> > Whenever I get a socket bind error, the JRE does not tell me which
> > socket
> > and port I am dealing with. Is anyone objected to a PR that would do
> > this:
> >
> > diff --git
> > a/httpcore5/src/main/java/org/apache/hc/core5/reactor/SingleCoreListe
> > ningIOReactor.java
> > b/httpcore5/src/main/java/org/apache/hc/core5/reactor/SingleCoreListe
> > ningIOReactor.java
> > index 91b1b22..9225bdb 100644
> > ---
> > a/httpcore5/src/main/java/org/apache/hc/core5/reactor/SingleCoreListe
> > ningIOReactor.java
> > +++
> > b/httpcore5/src/main/java/org/apache/hc/core5/reactor/SingleCoreListe
> > ningIOReactor.java
> > @@ -28,6 +28,7 @@
> > package org.apache.hc.core5.reactor;
> >
> > import java.io.IOException;
> > +import java.net.BindException;
> > import java.net.ServerSocket;
> > import java.net.SocketAddress;
> > import java.nio.channels.CancelledKeyException;
> > @@ -153,8 +154,8 @@
> > }
> > final SocketAddress address = request.address;
> > final ServerSocketChannel serverChannel =
> > ServerSocketChannel.open();
> > + final ServerSocket socket = serverChannel.socket();
> > try {
> > - final ServerSocket socket = serverChannel.socket();
> >
> > socket.setReuseAddress(this.reactorConfig.isSoReuseAddress());
> > if (this.reactorConfig.getRcvBufSize() > 0) {
> >
> > socket.setReceiveBufferSize(this.reactorConfig.getRcvBufSize());
> > @@ -167,6 +168,17 @@
> > final ListenerEndpoint endpoint = new
> > ListenerEndpointImpl(key, socket.getLocalSocketAddress());
> > this.endpoints.put(endpoint, Boolean.TRUE);
> > request.completed(endpoint);
> > + } catch (final BindException ex) {
> > + Closer.closeQuietly(serverChannel);
> > +// What I really want to say:
> > +// request.failed(
> > +// String.format("Socket bind failure for
> > socket
> > %s, address=%s, BacklogSize=%,d: %s", socket,
> > +// address,
> > this.reactorConfig.getBacklogSize(), ex),
> > +// ex);
> > + request.failed(new IOException(
> > + String.format("Socket bind failure for
> > socket %s,
> > address=%s, BacklogSize=%,d: %s", socket,
> > + address,
> > this.reactorConfig.getBacklogSize(), ex),
> > + ex));
> > } catch (final IOException ex) {
> > Closer.closeQuietly(serverChannel);
> > request.failed(ex);
> >
>
> Hi Gary
>
> Could you please put the changes you are proposing on a branch in
> GitHub? It would make it easier to review them.
>
Here you go: https://github.com/apache/httpcomponents-core/pull/117
Gary
>
>
> > The next question is: How about updating the APIs to change:
> >
> > org.apache.hc.core5.reactor.ListenerEndpointRequest.failed(Exception)
> > to:
> > org.apache.hc.core5.reactor.ListenerEndpointRequest.failed(String,
> > Exception)
> >
>
> What would be the benefit of adding another method that each and every
> instance of FutureCallback would have to implement? If you want to pass
> a more meaningful exception in one particular case why not wrapping the
> exception with more detailed context instead of breaking a very
> frequently interface?
>
> Oleg
>
>
> > This would be the same style you can see in Exception APIs where you
> > can
> > create an exception with both a messages and a cause. I would not
> > want both
> > failed(Exception) and (String, Exception), I think adding the one
> > param is
> > enough with cluttering the API.
> >
> > This also implies changing:
> >
> > org.apache.hc.core5.concurrent.BasicFuture.failed(Exception)
> > to:
> > org.apache.hc.core5.concurrent.BasicFuture.failed(String, Exception)
> >
> > and:
> >
> > org.apache.hc.core5.concurrent.FutureCallback.failed(Exception)
> > to:
> > org.apache.hc.core5.concurrent.FutureCallback.failed(String,
> > Exception)
> >
> > Thoughts?
> >
> > Gary
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>
