heremaps / oksse

An extension library for OkHttp to create a Server-Sent Event (SSE) client.
Apache License 2.0
229 stars 27 forks source link

Cannot add Accept: application/stream+json header #18

Closed lmorda closed 6 years ago

lmorda commented 6 years ago

My server guys are requiring me to use

Accept: application/stream+json

header to receive their Spring Flux server sent events. After tracing through the code, it appears that the prepareCall(Request request) method in the RealServerSentEvent class is forcing the request object to always have the same default headers for every call. This is preventing me from adding new headers to my ServerSentEvent object.

Request newRequest = request.newBuilder()
            .header("Accept-Encoding", "")
            .header("Accept", "text/event-stream")
            .header("Cache-Control", "no-cache")
            .build();

    call = client.newCall(newRequest);

I'm wondering if this is by design, since server sent events should be sent with the text/event-stream header. I think the reason they are requiring the application/stream+json header is that works in Google Chrome developer tools, which they are using for debugging.

lmorda commented 6 years ago
Request request = new Request.Builder().url(path).build();
request.addHeader("Accept", "application/stream+json");
OkSse okSse = new OkSse();
ServerSentEvent sse = okSse.newServerSentEvent(request, listener);

Interestingly, when the onClosed method is inevitably called, the headers on the ServerSentEvent object show that I've added the application/stream+json header, which I believe may be another bug. The actual headers on the SSE object used to make the call is not the same as the headers on the SSE object returned in the callback methods. That threw me off for awhile, because I was trying to debug viewing the headers in the onClose method:

@Override
public void onClosed(ServerSentEvent sse) {
    // sse request headers on this sse object actually includes 
    // the added Accept: application/stream+json header
}
marcelpinto commented 6 years ago

The protocol says that the header should have Accept text/event-stream. But I'll take a look

lmorda commented 6 years ago

Okay great, that's what I was thinking, that this was by design. I had them change the code so that both text event stream and application stream json are produced in the Spring Boot server. OkSse is now working in our implementation, thanks again for this awesome library!

@GetMapping( value = "v1/status/events", produces = { MediaType.APPLICATION_STREAM_JSON_VALUE, MediaType.TEXT_EVENT_STREAM_VALUE } )
public Flux<GeneralStatus> ...
marcelpinto commented 6 years ago

Glad to help. Even though I will take a look at the behavior you described.