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

Viewing Issue Advanced Details [ Jump to Notes ]

[ View Simple ] [ 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

Platform

Status

closed

Projection

none

OS Version

ETA

none

Fixed in Version

4.0.65

Product Version

4.0.64

Product Build

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

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.