SSE Output Wrong

[benjchristensen]

I am trying to create a Turbine server (https://github.com/Netflix/Turbine/tree/2.x) that outputs a stream to the Hystrix dashboard (https://github.com/Netflix/Hystrix/tree/master/hystrix-dashboard). The output when using ServerSentEvent does not work.

Here is the manually written code that works and then the one using ServerSentEvent that doesn't work.

// this outputs correct data
return response.writeStringAndFlush("data: " + JsonUtility.mapToJson(data) + "\n\n");
// this doesn't ...
// return response.writeAndFlush(new ServerSentEvent("", "data", JsonUtility.mapToJson(data)));

It appears related work was done in #141 but the comments in that one suggest it may be the cause of this issue.

Using ServerSentEvent it emits like this:

event: 
data: {"currentCorePoolSize":1070,"current ..

what is needed is:

data: {"currentCorePoolSize":1070,"current ..

[tbak]

The problem is with event field being empty string. Could you try this:

return response.writeAndFlush(new ServerSentEvent(null, "data", JsonUtility.mapToJson(data)));

If you look into the spec: http://dev.w3.org/html5/eventsource/#event-stream-interpretation, it seems that event field with no value is also valid.

field         = 1*name-char [ colon [ space ] *any-char ] end-of-line

[benjchristensen]

it seems that event field with no value is also valid.

What do you mean by that? I'm basing the "working" off of the fact that the browser is not correctly reading the stream when it has the event: in it.

[benjchristensen]

Using null still emits the event: text.

return response.writeAndFlush(new ServerSentEvent(null, "data", JsonUtility.mapToJson(data)));

that emits something I'd never expect:

event: data
data: {"rollingCountFa...

And this ...

return response.writeAndFlush(new ServerSentEvent(null, "", JsonUtility.mapToJson(data)));

emits ..

event:
data: {"rollingCountFa...

However, this one seems to emit the right thing:

return response.writeAndFlush(new ServerSentEvent(null, null, JsonUtility.mapToJson(data)));

Thus I'd argue that we should have an overload on ServerSentEvent that just takes the data. I also suggest that we should be able to just write a string and have it do the right thing instead of allocating a ServerSentEvent. I have chosen SSE as the protocol, so if I want to output "data" in SSE I should be able to just write a string and it show up correctly as data: <string>. Only if I need the "event" stuff should I need to use the ServerSentEvent object.

[tbak]

API is fine, but we could add check for empty string and not send anything if it is found:
If you look at the constructor:

public ServerSentEvent(String eventId, String eventType, String eventData) 

You should provide three values. They will be assigned to id, event and data fields respectively.
It would be good to have a builder here, given that there is one more field in the standard "retry".
Overladed constructor for just data field is good option as well.

[benjchristensen]

API is fine

The API is not fine if it takes me talking to the author of the library to use it and get expected results :-)

In 2+ years of using SSE I have never once needed eventId or eventType and it should not need to be discovered that one must pass in null, null to make those go away.

Using Chrome, I can not get an EventSource stream to work when eventId is set to something, but the following javascript works when I use null, null:

var es = new EventSource("http://localhost:7979/hystrix-dashboard/proxy.stream?origin=http%3A%2F%2Flocalhost%3A8888")
es.addEventListener('message', function(e) {
  console.log(e.data);
}, false);

I am no expert on the spec, I just know that the simple case of emitting "data:" only works when eventId does not exist and I don't know how to use SSE when eventId is there, and our current API forces a decision about it.

[tbak]

I have created an improvement issue for this (#222).

[benjchristensen]

Thank you.

[benjchristensen]

Also, how does the event field get used and work with Chrome? Are we emitting that correctly?

[tbak]

As I understand it, event field may contain any value which semantic is defined by the application. W emit it according to the spec. SSE standard is more precise about event id handling. For that I created an improvement issue some time ago: #153, but there was no discussion about when and how to implement it. We could leverage for that rx-netty-contexts.

[benjchristensen]

Not sure what that means ... can you show me Javascript code that runs in Chrome/Safari that correctly consumes the RxNetty output with eventIDs in it? I can't figure it out. When eventID is included as per the RxNetty implementation no data flows when using this code:

var es = new EventSource("http://hostname:port/rxnetty-server")
es.addEventListener('message', function(e) {
  console.log(e.data);
}, false);

[tbak]

I was looking into this, and I can only confirm that there is a problem with handling the SSE event stream generated by RxNetty example in the browser. If you look at the example from Mozilla: https://developer.mozilla.org/en-US/docs/Server-sent_events/Using_server-sent_events, we render the content in the very same way.

There is a note in the standard, that chunked encoding causes some problems for SSE stream:
http://dev.w3.org/html5/eventsource/#authoring-notes

Authors are also cautioned that HTTP chunking can have unexpected negative effects on 
the reliability of this protocol. Where possible, chunking should be disabled for serving 
event streams unless the rate of messages is high enough for this not to matter.

This is more about a delay in stream delivery.

I will try to find some existing SSE streams properly handled by the browsers to compare what we do differently. Do you know some?

[tbak]

Good description of handling of SSE events with custom event types is provided here: https://developer.mozilla.org/en-US/docs/Server-sent_events/Using_server-sent_events.
If event type is not defined, it defaults to "message", and an event handler 'onmessage' can be used, instead of addEventListener.
If event type is defined, a corresponding event listener must be installed to handle it.
Below client works for both Firefox and Chrome, with RxNetty HttpSseServer (slightly extended to generate two types of events). Event ids are ok as well.

"use strict";

function receiveMessages() {
    if (typeof(EventSource) !== "undefined") {
        // Yes! Server-sent events support!
        var source = new EventSource('/aggregator-api/message/stream');
        source.onmessage = function (event) {
            console.log(event);
            var data = event.data;
            var id = event.lastEventId;
            var newEntry = '<div class="message"> data only=' + data + '</span></div>';
            document.body.innerHTML = newEntry + document.body.innerHTML;
        };

        source.onopen = function (event) {
            // Connection was opened.
            console.log('opened')
        };

        source.onclose = function (event) {
            // Connection was closed.
            console.log('connection closed')
        };

        source.addEventListener("ping", function(event) {
                console.log(event);
                var data = event.data;
                var id = event.lastEventId;
                var newEntry = '<div class="message"> ping with id=' + id + ',and data=' + data + '</span></div>';
                document.body.innerHTML = newEntry + document.body.innerHTML;
            },
            false
        );
    } else {
        // Sorry! No server-sent events support..
        console.log('SSE not supported by browser.')
    }
}

window.onload = receiveMessages ;

[NiteshKant]

PR #266 reimplements SSE in the package io.reactivex.netty.protocol.http.sse and the following are the primary changes w.r.t this issue:

• Default ServerSentEvent constructor only emits a single data event.
• ServerSentEvent class provides three create methods withEventIdAndType, withEventType and withEventId to create events with eventId and eventType.
• Verified that one can directly write a string on an HttpServer configured to write SSE. This can be done using HttpServerResponse.writeString() method.
• Added another method HttpServerResponse.writeBytes() to also write a ByteBuf