Proper approach to close/disconnect

[mshick]

I'm pretty baffled by this. I can't seem to trigger a db.close() without getting an Error: unexpected disconnection. I've attached handlers to every event emitted by the db itself and the Net client connection, and it doesn't seem that I have any activities that are waiting to be done -- no drain or close events are triggered. I've even tried a simple setTimeout() to see if it's some sort of race condition.

I'm making a multilevel client with the approach in the readme -- Net.connect() -- and I do a simple connect then disconnect, without any activity in between, but still the error.

[juliangruber]

Can you verify that you do catch the error at db.createRpcStream()? I agree that this is unexpected behavior.

[dominictarr]

This is a mux-demux error: https://github.com/dominictarr/mux-demux/blob/master/inject.js#L73

The outer stream is being closed before the interior streams have all finished.

[mshick]

I've tried catching the error on the createRpcStream() and the Net.connect() stream, but it is still thrown.

I made a mistake in my first issue -- I was working within a test that was opening a writeStream and I didn't realize it.

It seems like if I do the following:

db.createReadStream({})
    .on('close', function () {
        db.close();
    });

I will get the error. In that case, if I wrap the db.close() in a process.nexTickt() things work correctly.

However in this case (what the module I'm working with was doing):

db.createReadStream({})
    .pipe(db.createWriteStream())
    .on('close', function () {
        db.close();
    });

Nothing I do seems to prevent the error.

I'm now wondering if this is because the writeStream isn't being properly closed, and that triggers the disconnect error?

If that seems to be the case, how do I close a writeStream like that? I've tried a few things, (writeStream.end() in the readStream.on('close')), but haven't had any luck.

Aside from that, should I consider the process.nextTick() approach the right way to close the connection if a readable stream was possibly opened?

Thanks!

[dominictarr]

what are you trying to achieve by closing the database? maybe there is a simple to get this to work the way you need it to.

[mshick]

I’ve been contributing to this project’s Level adapter: https://github.com/geddy/model https://github.com/geddy/model

And for that, I’m just trying to work through a test series for Multilevel, which includes a disconnect() method.

I actually just got it working by dropping the writeStream in favor of a batch operation, which is probably the right way to go anyway.

Still, I’d like to know generally what I need to do to ensure I can disconnect properly when using Multilevel — maybe I have a web server connected and want to disconnect on error as part of a shutdown procedure? Seems like good practice...

On Sep 18, 2014, at 6:19 PM, Dominic Tarr notifications@github.com wrote:

what are you trying to achieve by closing the database? maybe there is a simple to get this to work the way you need it to.

—
Reply to this email directly or view it on GitHub #60 (comment).

[dominictarr]

oh is it just that you just want the tests to exit?

[mshick]

Yes in this case — though I was also trying to do a db.close() in my own app code and encountering similar problems.

Maybe the simplest form of the question is, does a db.close() need to happen on the nextTick after opening a readStream?

Forgive me if this is some basic Streams stuff, I’m still fairly new to them.

On Sep 18, 2014, at 6:41 PM, Dominic Tarr notifications@github.com wrote:

oh is it just that you just want the tests to exit?

—
Reply to this email directly or view it on GitHub #60 (comment).

[dominictarr]

@mshick but what where you trying to do when you used close in your own code?

[juliangruber]

i think the problem is that we don't make a distinction between the connection breaking and a manual db.close()

[dominictarr]

yeah it should probably just wait for the open streams to end, like leveldown waits for the open iterators before closing

[juliangruber]

-> keep track of all open streams?

[mshick]

@dominictarr In my own code I was serving up data to an API. This was before I started using Model to abstract between Level and my API endpoints. I would do db.get() and db.createReadStream() (for browsing) at various points. I would issue db.close() on SIGINT, and noticed I got the error from time-to-time, apparently because the stream was open.

When I saw the error come up repeatedly in this test I figured I was just doing something wrong and opened this ticket.

I suppose it would be nice if db.close() could safely shut itself down by waiting for the streams to end, but I understand if that isn't practical.

It sounds like the correct approach is to keep track of and ensure my streams are closed before issuing a db.close().

I'm still puzzled by why the stream isn't sufficiently closed in the stream.on('end') event (hence, the whole nextTick() thing). Is that expected behavior?

Here's a gist that demonstrates what I'm talking about: https://gist.github.com/mshick/db8469e17ab767ad01b9

As-is it will always thow the disconnect err.

[jimkang]

I'm having trouble with this as well, also in a test context. (I worked around it by dropping tape (it complained about process.exit)) for it and just using process.exit at the end of the test. I tried closing every stream I think I opened.

Is there a convenient way to ask Node or V8 what streams or resources it's waiting on or just how to find out why a process is hanging around in general?

[dominictarr]

I think if you set this to false https://github.com/juliangruber/multilevel/blob/master/lib/server.js#L13
then it will kill the streams without emitting errors when the connection closes.

[juliangruber]

i think so too, but we did that line on purpose so it wouldn't swallow errors. ah, or do you mean setting .error = false just before closing the connection?

[dominictarr]

hmm, this is really a mux-demux problem. but to be honest, I am no longer using mux-demux.
Now I use https://github.com/ssbc/muxrpc instead.