0006326: com.caucho.remote.websocket.WebSocketWriter transforms all IOExceptions into IllegalStateExceptions

Viewing Issue Simple Details [ Jump to Notes ]

[ View Advanced ] [ Issue History ] [ Print ]

Category

Severity

Reproducibility

Date Submitted

Last Update

0006326

[Resin]

minor

always

02-14-20 06:03

03-11-20 15:07

Reporter

James Byatt

View Status

public

Assigned To

ferg

Priority

normal

Resolution

fixed

Status

closed

Product Version

4.0.64

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.

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

Attached Files

Relationships

There are no notes attached to this issue.

Issue History
Date Modified	Username	Field	Change
02-14-20 06:03	James Byatt	New Issue
03-11-20 15:07	ferg	Assigned To	=> ferg
03-11-20 15:07	ferg	Status	new => closed
03-11-20 15:07	ferg	Resolution	open => fixed
03-11-20 15:07	ferg	Fixed in Version	=> 4.0.65

Mantis 1.0.0rc3[^]

27 total queries executed.
25 unique queries executed.