search

ssbc / secret-stack

connect peers to each other using secret-handshakes
MIT License
90 stars 19 forks source link

Don't timeout RPC from self for inactivity #16

Closed clehner closed 6 years ago

clehner commented 7 years ago

My sbot started shutting off periodically. I am running it with ssb-party and patchfoo. I found that patchfoo's rpc connection was getting closed by scuttlebot, and then ssb-party would shut down scuttlebot since there were no more clients connected. This PR restores the earlier behavior, from before b659eecf3512660c786bac707b54b0c10107a68a, of allowing clients to remain connected indefinitely. Note that client connections from a master key (for example, as described in %9id5lKdgxl3ZleJa5BeMeAVLCnMwQepyntPLxisLnBE=.sha256) will still get closed by the inactivity timeout. To allow for those, clients will have to use a keepalive.

mmckegg commented 7 years ago

Ah! This explains a lot. I was wondering why realtime feeds mysteriously would stop updating. In patchcore these don't automatically reconnect.

Thanks @clehner!

dominictarr commented 7 years ago

oh, this is where tests are handy to stop you breaking something you weren't ment to. I had forgot about this, but I must have written or merged it at some point

clehner commented 7 years ago

@dominictarr i added a test. is this good?

dominictarr commented 7 years ago

great, merging

dominictarr commented 7 years ago

merged into 4.0.0 (with a fix)

Okay, I'm interpreting this as a breaknig change. the scuttlebot tests are broken for me right now, and I don't have time to fix them currently (it's something about test/plugins.js not working because the ssb/db/LOCK is still there) I think it's just something is weird because I went back to my old hard drive.

clehner commented 7 years ago

Thanks

clehner commented 7 years ago

It would have been better I think to have made the earlier change a breaking change, because timing out has broken multiple (all?) existing sbot client applications. This was also an issue a year or two ago I recall, and now we are back to the bad old days where scuttlebot client applications have to use a keepalive. This means even sbot(c) cli can't be used to pipe live changes into other processes. Is the test failure you mentioned a block on updating scuttlebot to use secret-stack@4?

dominictarr commented 7 years ago

That would have been better, I agree. We need to keep the scuttlebot tests passing, otherwise we have no idea what is happening. I'd be fixing them right now, except I am preparing for a talk this evening. (and I am doing another talk tomorrow) but if made a PR to that passes the tests and updates to secret-stack@4, I would totally be merging! They certainly worked before I last published (because I have a prepublish script that will prevent publishing if they don't work) I just don't have time to fix them for the next couple of days.

arj03 commented 6 years ago

I guess this one can be closed now @dominictarr? Also 4.0.0 was pushed to npm, but this repo is missing the version bump.