whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.16k stars 2.69k forks source link

EventSource close #3380

Closed PIPOGit closed 6 years ago

PIPOGit commented 6 years ago

Hi.

I'm working with some EvenSource code, from both: client and server side, and I've found that "close()" method, form Client-side, does NOT communicate the "closed-status" to the server side, leading to a repeatable server-sided errors.

I.e.: Using Apache Tomcat 9 and Spring 5, when from JavaScript I call closing method ("eventSource.close();") this information does NOT reaches the Spring-based application in the server-side, so, the nest time the code in the server side tries to send an event thru all their registered emitters ("SseEmitter.class"), Apache throws a pair of exceptions: "IllegalStateExceptoin" and "IOException", dealing with the fact that the connection is closed/broken.

So, whenever Spring tries to send an event over a "client-sided closed connection", Apache show errors. And the log file increases.

So, I propose that "EventSource.close()" Javascript-spec method SENDS something to the server in order register this behaviour to try to avoid these annoying server-side logs.

Thanks for all.

annevk commented 6 years ago

This is a bug in Spring then, that it does not know about the connection being closed. We cannot really do anything else than close the connection that's HTTP/1 compatible. I recommend filing a bug against the framework.

PIPOGit commented 6 years ago

Should be a "bug" if Spring would know that the connection is closed. BUT; "EventSource.close()" does NOT send anything to the server; so that, in the server-side, no-one will know a connection is closed by the client. That's the problem.

In fact, the problem is reported, to Spring-based apps, by Apache Tomcat, who rises the main exception server-side.

annevk commented 6 years ago

That means Apache Tomcat knows the connection is closed, no?

PIPOGit commented 6 years ago

No. Neither Spring nor Tomcat, nor any one.

annevk commented 6 years ago

Why would it throw exceptions then?

PIPOGit commented 6 years ago

Because HTTP server keeps the original connection opened... until someone tells him to close!

annevk commented 6 years ago

Yeah, but again, the client has no way of informing the server. So the server better have some logic in case of an unexpected close and deal with that.

PIPOGit commented 6 years ago

That's what I said before: "EventSource.close()" should "send" some kind of signal to the server to tell him "Guy, I'm disconnecting!". :)

annevk commented 6 years ago

How though? There's no way to do that in HTTP.

PIPOGit commented 6 years ago

Just in the same way the client "opens" a connection, sending "something" to the server. Closing the connection should be something similar.

( I mean, HTML page or browser, may be one -or both- of them ) If the client opens the connection, the same client should "close" the connection also.

annevk commented 6 years ago

There's no signal for closing though.

PIPOGit commented 6 years ago

I know! :^) But "should be!". That's what I'm telling.

annevk commented 6 years ago

Actually, I'm wrong, when the client closes the connection presumably it sends a FIN message on the TCP layer.

PIPOGit commented 6 years ago

Well.. I hope so! ;^)

Anyway; I'm opening a bug report in Tomcat to see whether it's "their bug". I don't know the underlying implementation of the protocol, but someone should say: "The connection is over!" and manage it properly.

Let's see Tomcat guys... :)

Thanks, Anne.

PIPOGit commented 6 years ago

Opened: https://bz.apache.org/bugzilla/show_bug.cgi?id=62028

martin-g commented 6 years ago

Tomcat, or any other web server, doesn't know that the client is no more there! The application code (Spring app) tries to send more messages and this fails with "connection already closed" error and then Tomcat releases its resources for this connection at the server side. Same might happen for any static resource - it the user navigates to another page while a static resource is still being served such kind of error will occur.

In this case a workaround might be to listen for beforeunload on the window and use EventSource#close() to notify the server.

I am not sure whether the browsers should do this by default.

PIPOGit commented 6 years ago

OnHTML page: <body ... onUnload="Close()"> OnJS: function Close() { [eventSource].close(); }

... but nothing is sent; it's just something to tell the browser (from the JavaScript client Web App) to release the WebSocket resource at browser level.

martin-g commented 6 years ago

Try with beforeunload. unload is too late.

window.addEventListener("beforeunload",function() {
     eventSource.close();
});
PIPOGit commented 6 years ago

Well... but actually, it's not a problem of the JavaScript event, but the connection itself. :)

martin-g commented 6 years ago

At the time of beforeunload event the connection should be still opened. By calling close() you tell the application at the server side to stop writing new events because the client is disappearing.

domenic commented 6 years ago

So, from what I can tell, the connection is closed; that signal itself is received by Tomcat, as it's part of HTTP (and TCP) and Tomcat certainly knows things are closed enough to give you error messages. You should ask Tomcat to expose that low-level signal that it's receiving to your application code, so that you can get notified when HTTP connections close.

In other words, it would be redundant for the browser to send two signals: "1) I'm going to close the connection" + "2) close the connection". Your server already understands 2), otherwise you would not be getting any error messages.

So, I don't think we should change the spec here. But I encourage you to take this up with Tomcat maintainers. For example, this functionality is already provided in Node.js: see https://stackoverflow.com/a/3954041/3191. Asking Tomcat for feature parity with Node.js seems reasonable to me.

Let's close, as I don't believe this is actionable for the spec. But we're happy to continue discussing in the closed thread.