Ability to reset internal `ended` state
Closed this issue · 5 comments
Just wondering if this would be something that is useful or just plain dangerous. I've tracked down a bug in rtc-signaller
(which is really obvious now that I'm looking at it) - see rtc-io/rtc-signaller#35 and it could definitely be solved by allowing me to either:
a. reset the internal ended
state of the stream
b. create a new pull-pushable instance transferring the buffer from an old instance
Amongst other things... thoughts? Happy to make a PR if you think either makes sense.
Actually to clarify, I have been able to fix the issue with no changes to pull-pushable, but believe there is probably a case where buffered messages will go missing without implementing something like this. Not sure if it is worth making a change to pull-pushable though as in reality it does exactly what it should do...
I don't quite understand what you are getting at - do you mean 1) to mark the stream as ended, but if they havn't read everything you are still able to append more items?
Or do you mean 2) to reuse a stream instance that has ended?
- sounds maybe reasonable but a bit weird. 2) is out of the question though.
Oh, I think the problem here is that websockets don't have half duplex endings... there is "close" but that is the same as hanging up the phone (either party may hang up, and then neither party can speak)... humans streaming voice into analogue telephony systems found this behavior to be arkward, so they developed a disconnection handshake (I say "goodbye" and wait to get "goodbye" from you, and then either of us can hang up)
I implemented that as a module, specifically for use with websockets pull-goodbye
Apologies, I don't think I explained this very well at all. I'm not suggesting 1, and I completely agree the 2 is just plain silly (though it is what I suggested in the original issue via suggestion a).
I don't think pull-goodbye
is the solution in this particular case (though I could well be wrong), as I'm catering for a websocket being dropped by the server so don't have a polite hangup option.
The scenario is this:
SRC1 -> THR1 -> SNK1
Where SRC1
is a pull-pushable instance, THR1
is a pull.Through
stream which is monitoring a stream end condition and reconnecting when appropriate, SNK1
is a pull-ws
Sink. Now in the case where SRC1
has already buffered messages [A, B, C]
and successfully sent [A, B]
before being rudely disconnected by the server. At this point I rebuild the pull-stream flow with:
SRC2 -> THR2 -> SNK2
SNK2
is starting the websocket connection process, and SRC2
is already to go buffering messages and once SNK2
is ok to go those newly buffered messages will start to flow though. The problem is what happens to the buffered message C
which was originally queued in SRC1
(now a distant memory). Well at this stage, that message is lost, and while it practically never happens I reckon it probably will at some stage.
I think what I want to be able to do is be able to initialise a new pull-pushable
instance, such that any queued messages in SRC1
could be transferred to SRC2
, something like:
// create src1
src1 = pushable();
src1.push('A');
src2.push('B');
src3.push('C');
// consume two items from src1 and have it end
...
// initialise a new source cloning any remaining items in src1
// taking into acount the current first arg of creating a pushable is the `onClose` handler
src2 = pushable(null, src1.clone());
That's what I was thinking anyway, which I think makes sense, though I am prone to nonsensical thought...
right! okay I get it you want to have transparent reconnections - this means a client needs to reconnect and then request the log from the last point it got something - it's not good enough for the server to remember what it sent last, because the client may have crashed. or packets may have been dropped.
basically, everything needs a sequence or log number... Quite often there are sequences numbers in the application layer that makes this easy. you could certainly have a queue too, that remembered a buffer of items and the client would send acks to tell the server which things it could forget...
Yeah thankfully @briely has implemented all this stuff on a newer more robust signaller implementation :)
Closing this for now, because like you say this is probably an application layer concern.
Thanks Dominic :)