Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling multiple HTTP based protocols on a single server. #202

Closed
NiteshKant opened this issue Aug 11, 2014 · 16 comments
Closed

Handling multiple HTTP based protocols on a single server. #202

NiteshKant opened this issue Aug 11, 2014 · 16 comments
Milestone

Comments

@NiteshKant
Copy link
Member

In some offline discussions and on pull request #196 it came to the fore that serving multiple higher level protocols (SSE, Websockets, plain request-response) over HTTP on a single HTTP server is not very intuitive.

The following is the usecase:

  • Lets assume we have a single HTTP server on port 8080
  • URI /myapp/ws serves Web sockets traffic.
  • URI /myapp/sse serves SSE traffic.
  • URI /myapp/req-resp serves a simple HTTP based request-response traffic.

In order to implement a RequestHandler for the above routing capabilties, one would have to do something like:

 new RequestHandler<ByteBuf, ByteBuf>() {
     @Override
     public Observable<Void> handle(HttpServerRequest<ByteBuf> request,
                                    final HttpServerResponse<ByteBuf> response) {
         String uri = request.getUri();
         if (uri.startsWith("myapp/sse")) {
             ChannelPipeline pipeline =
                     response.getChannelHandlerContext().pipeline();
             // Configure pipeline for SSE
         } else {
             printRequestHeader(request);
             response.writeString("Welcome!!");
             return response.close(false);
         }
     }
 }

There are multiple issues here:

  • The above is non-intuitive i.e. how to get a handle of netty's pipeline.
  • The existing PipelineConfigurator implementations assume that they are only called at the creation of the pipeline and not incrementally during request processing. eg: The SSE configurator is:
        serverPipelineConfigurator.configureNewPipeline(pipeline);
        pipeline.addLast(SSE_ENCODER_HANDLER_NAME, SERVER_SENT_EVENT_ENCODER);
        pipeline.addLast(SSE_RESPONSE_HEADERS_COMPLETER, new ChannelOutboundHandlerAdapter() {
            @Override
            public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception {
                if (HttpServerResponse.class.isAssignableFrom(msg.getClass())) {
                    @SuppressWarnings("rawtypes")
                    HttpServerResponse rxResponse = (HttpServerResponse) msg;
                    String contentTypeHeader = rxResponse.getHeaders().get(CONTENT_TYPE);
                    if (null == contentTypeHeader) {
                        rxResponse.getHeaders().set(CONTENT_TYPE, "text/event-stream");
                    }
                }
                super.write(ctx, msg, promise);
            }
        });

It configures the underlying (HTTP) configurator first.

  • The pipeline.addLast() semantics will not work as RxNetty adds its own handlers at the end of the pipeline and that is an invariant that has to be maintained in order for the callbacks to work correctly.
@NiteshKant NiteshKant added this to the 0.3.12 milestone Aug 11, 2014
@NiteshKant
Copy link
Member Author

\cc @gorzell @tbak

@benjchristensen
Copy link
Member

👍 on needing this ability. We will need HTTP and WebSockets on our servers.

@NiteshKant NiteshKant modified the milestones: 0.3.12, 0.3.13 Aug 16, 2014
@NiteshKant
Copy link
Member Author

Moving to 0.3.13 milestone.

@NiteshKant NiteshKant modified the milestones: 0.3.13, 0.3.14 Sep 7, 2014
@NiteshKant NiteshKant modified the milestones: 0.3.14, 0.3.15 Sep 19, 2014
@tlockney
Copy link

tlockney commented Oct 9, 2014

I originally opened a new issue (#241, which I have since closed) for another use case that needs to be better addressed. That is, the case where you simply need/want to compose different request handlers together. This came up when I was trying to find a simple way to compose a simple set of RESTful endpoints alongside a set of static files. The hack was to make our existing handler delegate to a WebappFileRequestHandler instance when none of REST paths are matched, but this seems a less than ideal solution in the long run.

Another common, related scenario I might have is to want to encapsulate common endpoints (e.g., healthchecks, monitoring/admin services, etc.) into a library and then pull those into various webservices. Again, this would require modifying the webservices handler to either match on those various paths and then delegate to the appropriate handler, or to use the fall-through approach I used for the static files scenario, which only works if there's one fall-through option rather than many.

@NiteshKant
Copy link
Member Author

@tlockney karyon, which is built on top of RxNetty and is meant to be a higher level abstraction for writing applications, contains a SimpleUriRouter which demonstrates how to compose handlers based on different URI expressions. Is this close to what you are looking for?

@tlockney
Copy link

@NiteshKant Sure, I'm aware of that class. In our case, we're not using Karyon, but I used a somewhat similar idea (hough much more basic since this was a one-off issue. But it made me wonder about the general solution to this problem, hence my additions to this thread. Looking at SimpleUriRouter again, though, I wonder if it would make sense to pull something similar to that into RxNetty directly.

@jkschneider
Copy link

+1. Like @tlockney, I wanted a way to compose static content and a simple REST endpoint. I find the most attractive characteristic about rx-netty is its low number of external dependencies compared to competitors. Adding karyon limits this benefit.

@jkschneider
Copy link

I think it is worth noting here that there is a nasty bug somewhere in the holding of state around queryStringDecoder in HttpKeyEvaluationContext. This state seems to cross uri boundaries. It doesn't appear that there are any tests for it in Karyon, and it may not be worth tracking down if the PR here obviates the need for SimpleUriRouter altogether.

@jkschneider
Copy link

@NiteshKant - I would be happy to write up wiki docs for this feature if you like the implementation.

@jonashall
Copy link

+1 Really need this support. In the meantime I'm trying to reconfigure the pipeline to support WebSockets on certain uris. Anyone who managed to get this working?

@chen56
Copy link

chen56 commented Jan 7, 2015

+1 : Configure pipeline

@kromit
Copy link

kromit commented Jan 7, 2015

+1: really missed feature

@NiteshKant
Copy link
Member Author

This will be available in 0.5.0 milestone.

@NiteshKant NiteshKant added this to the 0.5.0 milestone Jan 8, 2015
@NiteshKant
Copy link
Member Author

This is fixed for 0.5.x, an example of this is available here and following is the relevant section:

HttpServer.newServer().start((req, resp) -> {
                               if (req.isWebSocketUpgradeRequested()) {
                                   return resp.acceptWebSocketUpgrade(wsConn ->
                                                                              wsConn.writeAndFlushOnEach(wsConn.getInputForWrite()));
                               } else if (req.getUri().startsWith("/hello")) {
                                   return resp.writeString(Observable.just("Hello World"));
                               } else {
                                   return resp.setStatus(HttpResponseStatus.NOT_FOUND);
                               }
                           });

@chen56
Copy link

chen56 commented Jun 3, 2015

@NiteshKant When can release the 0.5 version, I have been waiting for a long time

@NiteshKant
Copy link
Member Author

@chen56 0.5.x is a major release, most of the parts of rxnetty have been re-worked, that is the reason for the delay. I am expecting to release a snapshot next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants