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

handling disconnect/reconnect #42

Open
timoxley opened this issue Dec 4, 2013 · 8 comments
Open

handling disconnect/reconnect #42

timoxley opened this issue Dec 4, 2013 · 8 comments

Comments

@timoxley
Copy link

timoxley commented Dec 4, 2013

I can't seem to make reconnection work simply. This issue is probably a continuation of #9.

In the multilevel server/client there's a MuxDemux({error: true}) introduced by @soldair, presumably as part of getting reconnection to work (though I can't find the issue/PR where this change was introduced).

Curiously, this change actually breaks my use of reconnect with multilevel? If I pull out this error: true, reconnection works as expected. Also not sure where to even attach the error event listener to capture the "unexpected disconnection" error.

Sample:

// client.js
var reconnect = require('reconnect-engine');
var multilevel = require('multilevel')
var manifest = require('./manifest.json')
var db = multilevel.client(manifest)

reconnect(function(con) {
  con.pipe(db.createRpcStream()).pipe(con)
  db.liveStream().on('data', function(message) {
    console.log('message', message)
  })
})
.connect('/engine');
@dominictarr
Copy link
Collaborator

unexcepted disconnection is on the inner muxdemux streams, fixing that probably needs a pull-request to multilevel. it's emitted when the outer stream has ended/closed/errors, but there are inner streams that have not yet ended, but can no longer send any data because the parent stream is dead.

@timoxley
Copy link
Author

timoxley commented Dec 4, 2013

ok i've figured why I can't seem to catch this error, it's coming out of the livestream. This fixes it:

...
reconnect(function(con) {
  con.pipe(db.createRpcStream()).pipe(con)
  db.liveStream().on('data', function(message) {
    console.log('message', message)
  }).on('error', function() {
    // ignore error I guess?
  })
})
.connect('/engine')

Perhaps there's some way I can shield the db from disconnections/reconnections altogether?

@juliangruber
Copy link
Owner

the problem is that ideally the livestream would be reconnect aware, but currently isn't. So on error you should create a new live stream.

@timoxley
Copy link
Author

timoxley commented Dec 4, 2013

@juliangruber what needs to be added to livestream to make it reconnect aware?

@juliangruber
Copy link
Owner

oh damn i forgot it's internally only using streams for initial data, not for live updates. hmm maybe we could emit db.on('error') when the connection aborts and when level-live-stream sees that it can do its resuming logic

@dominictarr
Copy link
Collaborator

so, the thing with reconnect is that there is multiple ways you could resume a stream,
and each specific method needs to be aware of that way.

for example, if you have an append only data stream, that is very easy to resume.
You just request the latest sequence number/timestamp and the stream can continue as if it's not errored. on the other hand, if you have a stream that reads the keys in order, and isn't live you can resume that too, using a similar technique.

But if you have an non-appending data source, and you are streaming it live, then you'd need to be a bit more clever to resume... you could cache some of the recent updates, and resend them if the timeout is short. But if the timeout was longer, you'll just have to resend all the data, and handle it immutably (or you could filter it on the server, if there are timestamps in the data)

This is really the problem, is there is no way you could do reconnections in general. it's coupled to the style of data that you have.

What kind of data do you have?

@dominictarr
Copy link
Collaborator

okay: i have an idea for this.

we make a small extension to level-manifest so that 1) it now generates a .js file instead of a .json and 2) support adding a function each manifest property.

this function wraps the stream call on the client, so that it can remember what ever it needs to reconnect (on that type of stream/data)

it might look like this:

methods: {
  liveStream: {
    type: 'readable',
    reconnect: function (data, args) {
      //this is called on each piece of data that comes out of the stream.
      //the function should look at the data, and then update the args,
      //which will be the args passed to the new instance of the stream that is created to reconnect.
      //e.g.
      var opts = args[0] //[{start, end}
      opts.start = data.key
      return args
    }
  }
}

methods that are lacking a reconnect method should emit an error instead,
also, it would be possible to have an option an async calls retry: true that
told multilevel to just retry that call, although, you'd certainly want a timeout on this,
and I think in many cases, it would probable be better to just error and allow the app to handle that.
a timeout on the stream reconnects may be desirable too.

@timlesallen
Copy link

timlesallen commented Aug 29, 2016

I am getting a Uncaught Error: unexpected disconnection when the server is not there and we try to connect and use multilevel. I believe this issue relates to my problem. Anyone have a tip to work around this?

Update: it would seem that for the case I am seeing, I'm getting a connect event from the net.Socket. The server is not there, but somehow it thinks it has a connection (the box I am trying to connect to is in ec2). Then, when we actually try to use the db, that's when things are exploding...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants