Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol-level connection errors result in CLOSED status instead of ERROR #126

Open
ada-waffles opened this issue Apr 26, 2021 · 0 comments
Labels
needs triage Issue/PR needs triage by a project maintainer

Comments

@ada-waffles
Copy link

Expected Behavior

When a connection is closed due to a protocol-level error (such as rejected SETUP), the connection status should be set to ERROR.

As per the documentation here:

* - CLOSED: when the connection has been explicitly closed via `close()`.
* - ERROR: when the connection has been closed for any other reason.

Actual Behavior

Protocol-level connection errors result in the connectionStatus() Flowable emitting a status of CLOSED.

Steps to Reproduce

Minimal reproduction: https://github.com/duke9509/rsocket-js-error-status-repro

The server just immediately rejects every SETUP frame.

Output from the client:

Attempting to connect to server...
Initial connection established. SETUP frame sent.
Connection status: { kind: 'CONNECTED' }
Connection status: { kind: 'CONNECTED' }
Connection status: { kind: 'CLOSED' }

(I have no idea why CONNECTED is seemingly emitted twice, but I'm not too concerned about it and it seemed orthogonal to this issue so I didn't bother to dig into it)

Possible Solution

I can think of at least a couple approaches to fixing this. The simplest in my mind is to ammend DuplexConnection#close and its implementations to accept an optional error argument indicating that the connection is being closed due to an error and should behave as such.

This seems easy enough to implement and I've already tested it locally, so I plan to submit a PR.

Your Environment

  • RSocket version(s) used: 0.0.25
  • Other relevant libraries versions (eg. netty, ...): N/A
  • Platform (eg. JVM version (javar -version) or Node version (node --version)): Node 15.14.0
  • OS and version (eg uname -a): macOS Big Sur 11.2.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Issue/PR needs triage by a project maintainer
Projects
None yet
Development

No branches or pull requests

2 participants