Mantis - Resin
Viewing Issue Advanced Details
ID: Category: Severity: Reproducibility: Date Submitted: Last Update:
6326 minor always 02-14-20 06:03 03-11-20 15:07
Reporter: James Byatt Platform:  
Assigned To: ferg OS:  
Priority: normal OS Version:  
Status: closed Product Version: 4.0.64  
Product Build: Resolution: fixed  
Projection: none      
ETA: none Fixed in Version: 4.0.65  
Summary: 0006326: com.caucho.remote.websocket.WebSocketWriter transforms all IOExceptions into IllegalStateExceptions
Description: Hello,

We've written an application that makes use of resin's websockets, and we have some sending code that looks something like this:

WebSocketContext context = ... // we're called back with this object when the websocket starts
try (final PrintWriter writer = context.startTextMessage())
{
   writer.print("Hello world!")
}

Now - this usually works - unless the client at the other end has disconnected in the meantime. If that has happened, the root socket connection throws a SocketException, which eventually bubbles up into WebSocketWriter...which rethrows it as an IllegalStateException (!). Looking at the source, WebSocketWriter appears to do this everywhere it can. Here's its flush method, for example:

  @Override
  public void flush()
  {
    try {
      complete(false);

      _os.flush();
    } catch (IOException e) {
      throw new IllegalStateException(e);
    }
  }

This makes it very difficult for client code to discern problems caused by changes in connectivity from, well, anything else. I can sort of see why this choice would be made - PrintWriter hides IOExceptions behind a `trouble` boolean, and people might not know to call `checkError`. I'd argue that's their problem, not resin's, and behaving in what to me feels non-idiomatic makes things worse.

I'm not sure what the solution is here. Here, though, are some suggestions you should feel free to ignore :-)

- Maybe provide Writer instead of PrintWriter from startTextMessage?
  Breaks backwards compatibility :-(
  Could provide a different call, deprecate the existing startTextMessage, advise users, etc?
- Throw a well documented exception type other than IllegalStateException?
  Still a bit icky
- Something else?

Happy to discuss this further, be corrected, etc - let me know!

Thanks,
James.

P.S We're using 4.0.62, but a quick check of 4.0.64's source tells me that nothing has changed.
Steps To Reproduce:
Additional Information: Here's a stack trace:

java.lang.IllegalStateException: com.caucho.vfs.ClientDisconnectException: SocketStream[null]:java.net.SocketException: Socket closed
    at com.caucho.remote.websocket.WebSocketWriter.close(WebSocketWriter.java:213)
    at com.caucho.remote.websocket.WebSocketPrintWriter.close(WebSocketPrintWriter.java:66)
        ...killed a few frames of our calling code here...
Caused by: com.caucho.vfs.ClientDisconnectException: SocketStream[null]:java.net.SocketException: Socket closed
    at com.caucho.vfs.ClientDisconnectException.create(ClientDisconnectException.java:80)
    at com.caucho.vfs.SocketStream.write(SocketStream.java:295)
    at com.caucho.vfs.WriteStream.flush(WriteStream.java:400)
    at com.caucho.remote.websocket.WebSocketWriter.close(WebSocketWriter.java:210)
    ... 8 more
Caused by: java.net.SocketException: Socket closed
    at java.net.SocketOutputStream.socketWrite(SocketOutputStream.java:118)
    at java.net.SocketOutputStream.write(SocketOutputStream.java:155)
    at com.caucho.vfs.SocketStream.write(SocketStream.java:292)
    ... 10 more
Relationships
Attached Files:

There are no notes attached to this issue.