stack overflow when installing live-stream on a root db which has sublevels

[thlorenz]

Let's say on the server you have something like:

var db = sublevel(db);
var foosub = db.sublevel('foo');

// incorrect
liveStream.install(db);

// correct would be
// liveStream.install(foo);

Consuming on the client side like this:

// incorrect
db.liveStream().on('data', console.dir);

// correct would be
// db.sublevel('foo').liveStream().on('data', console.dir);

Since we did this incorrectly (i.e. attached live stream to root level instead of to the sublevels separately) we'll get a stack overflow error in 'json-buffer' due to the _parent on the sublevels which causes a circular reference:

...../node_modules/multilevel/node_modules/mux-demux/node_modules/json-buffer/index.js:34
    return JSON.stringify(/^:/.test(o) ? ':' + o : o)
                               ^
RangeError: Maximum call stack size exceeded

The solution here is to document to not do that and/or detect this earlier and throw a more meaningful error.

[dominictarr]

I'm still confused how this was happing, I want to get to the bottom of this! Where you doing liveStream.install in the client?

[thlorenz]

No, was doing that on the server. If you want to wade through the now fixed code, here is the server side and client side.

As I said, it is fixed now, before I just had liveStream.install(db) server side and db.liveStream().on('data', ...) client side.

Hope that helps. I was basically using it wrong, but if it is very important to you I can try to repro this with a smaller script tomorrow.

[dominictarr]

I'm mostly aiming to debug the documentation, or make it fail with a more obvious error message.

A reproduce script would be appreciated

[thlorenz]

As this script shows, this is not an isolated level-live-stream problem, but most likely due to some deserialization going on in muxdemux when used with multilevlel.

@juliangruber should I take this issue over there?

[juliangruber]

@thlorenz your script doesn't use multilevel, shouldn't it rather be a sublevel or level-live-stream problem then?

[thlorenz]

@juliangruber yes it's not using multilevel, that is why it doesn't repro the problem. Which is what led us to believe that this is not an isolated live-stream/sublevel problem. If you look at the stack trace above you'll see that my app blows up in json-buffer triggered by mux-demux.

It may not be too important since I can work around this by subscribing to the live stream on the actual sublevels instead the root db.

My question was just if we should move this issue to multilevel and I'll try to get around creating a script that isolates this issue as much as possible (it would use multilevel).

[juliangruber]

ah, ok I thought your script reproduced the problem. Sounds good to move over then.

[soldair]

hey guys here is a reduced test case.

https://github.com/soldair/level-live-stream_issues_7

https://github.com/soldair/level-live-stream_issues_7/blob/master/test/test.js

This is absolutely a bug that must be fixed. The client should never be allowed to take down the database server! Also just because you have sublevels doesn't mean every consuming client has to have logic using the fact that they exist.

the issue at hand is where to fix it!

• delete prefix key when passing the change to the root db in level-hooks?
• delete the prefix key before emitted from pull-level?
• delete this property before it's emitted in data from level-live-stream?
• should sublevel.toJSON return undefined null or the prefix? this is supported by json-buffer
• i dont think that cyclical structure detection should be a safety net in the serializer really.

the super hack i have to run now is

var through = require('through');

module.exports =  function fix(db) {

  var oldLiveStream = db.liveStream;
  if(!oldLiveStream) return;

  db.liveStream = function(){
    return oldLiveStream.apply(this,arguments).pipe(through(function(data){
      delete data.prefix;
      this.queue(data);
    })) 
  };  

  Object.keys(db.sublevels).forEach(function(k){
    fix(db.sublevels[k]);
  }); 
}

i'm real interested in getting a pull request in for this suggestions on the correct place to patch would help a lot.

[dominictarr]

deleting the prefix is just a hack if we don't actually know what is causing the problem. I think this is something to do with level-post, which basically adds post hook functionality a second time, since you are not exposing the sublevel the first time.

[dominictarr]

aha, I think the right fix is to have level-hooks compact the prefix to a string (which is also supported) but leave it there.

[soldair]

i can make that pr now! thanks for the advice like i said i want to remove my super hack asap

[dominictarr]

okay this is fixed - reinstall level-sublevel (to get level-hooks@4.4.5) and it should work.

[dominictarr]

can you make a PR that adds this as a test to level-sublevel?

all it needs to test for is that post hooks are always jsonifiable, even when using prehooks like this. Oh, by the way, you can also do: prefix: subdb.prefix() and that will work even with the old code.

[soldair]

thanks! i just saw that you supported it and it was just too nice not to use

[dominictarr]

it's best to use .prefix() since that works when you do batches even from the multilevel client.