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

Add a test where clients are disconnected forcefully #201

Merged
merged 3 commits into from
Apr 26, 2021

Conversation

cryptix
Copy link
Member

@cryptix cryptix commented Apr 26, 2021

This new test is just like the normal make a tunnel through the server with two js clients-test.

The only difference is: just after the tunnel is build and the clients are about to sign off happily, the server cuts the connection (by simply shutting down).

With the bug we isolated in #190, this leads the nodejs processes to exit immediately because of an uncaught throw in pull-stream drain.

To make these tests pass ssb-room-client needs to be patched here to have an extra done callback on the drain call.

Additionally we advise for pull-stream to have a better way of signaling the inability to handle the error then just throwing the error from the network without an indication who called drain without the optional 2nd callback. @staltz gave me this idea and I managed to find the culprit in room-observer.ts with it:

diff --git a/sinks/drain.js b/sinks/drain.js
index 2025462..ef8c6fc 100644
--- a/sinks/drain.js
+++ b/sinks/drain.js
@@ -3,6 +3,11 @@
 module.exports = function drain (op, done) {
   var read, abort
 
+  var causeErr
+  if (typeof done === 'undefined') {
+    causeErr = new Error('no done callback supplied')
+  }
+
   function sink (_read) {
     read = _read
     if(abort) return sink.abort()
@@ -18,8 +23,10 @@ module.exports = function drain (op, done) {
             if(end = end || abort) {
               loop = false
               if(done) done(end === true ? null : end)
-              else if(end && end !== true)
+              else if(end && end !== true) {
+                console.warn(causeErr)
                 throw end
+              }
             }
             else if(op && false === op(data) || abort) {
               loop = false

TODO: publish updated nodejs deps and update our package.json to use them.

@staltz
Copy link
Member

staltz commented Apr 26, 2021

I'm working on a fix to ssb-room-client, will let you know when you can update the version here

@cryptix cryptix linked an issue Apr 26, 2021 that may be closed by this pull request
@cryptix cryptix added the Bug Something isn't working label Apr 26, 2021
@cryptix cryptix added this to the NGIp milestone milestone Apr 26, 2021
@staltz
Copy link
Member

staltz commented Apr 26, 2021

Ship it

@staltz
Copy link
Member

staltz commented Apr 26, 2021

✔️

@cryptix cryptix merged commit ae1fd47 into master Apr 26, 2021
@cryptix cryptix deleted the test-against-190 branch April 26, 2021 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

muxrpc client crashes when go-ssb-room closes or something
2 participants