search

ssbc / ssb-ebt

secure scuttlebutt replication with epidemic-broadcast-trees
MIT License
18 stars 10 forks source link

change peerStatus to async to avoid error, add more careful checks on clock obj #11

Closed mixmix closed 6 years ago

mixmix commented 6 years ago

see this issue : https://github.com/ssbc/ssb-ebt/issues/10

problem with muxrpc throwing an error about not being given a callback on a sync function D:

also some better checks to avoid "can't find @ye+4.... on null" type errors from trying to dig into a non-existent object

cc @dominictarr

dominictarr commented 6 years ago

even if it is sync, you still have to provide a cb if it's remote, but it allows you to call it locally without a callback.

mixmix commented 6 years ago

oh, that's unexpected ... is this a muxrpc pattern? I guess that makes sense in a way, but then it's technically async isn't it?

So your reply is informative but it doesn't 100% clarify any direction for this PR / issue. Can I guess you're proposing :

???

dominictarr commented 6 years ago

yes this is a muxrpc pattern. it's sync on the server side, but because anything that goes from the client to the server is async, if it's called from the client it's async.

There are other a number of other sync methods such as whoami, getAddress, query.explain.

my suggestion is to use a callback as per the docs

mixmix commented 6 years ago

Cool, didn't know about that pattern, had assumed the docs were wrong in calling it sync. That it's async makes sense. I think it would be less confusing if all methods were just async or source, because caking methods in the server isn't that common... And if you are (eg in a plugin) having the same method behavior would feel more cohorent (this is some of the point of rpc pattern right?)

On Sun, 27 May 2018, 11:39 Dominic Tarr, notifications@github.com wrote:

yes this is a muxrpc pattern. it's sync on the server side, but because anything that goes from the client to the server is async, if it's called from the client it's async.

There are other a number of other sync methods such as whoami, getAddress, query.explain.

my suggestion is to use a callback as per the docs

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ssbc/ssb-ebt/pull/11#issuecomment-392294784, or mute the thread https://github.com/notifications/unsubscribe-auth/ACitnijjb7ZRFcaAMN_Z6vcur_uvEPfYks5t2efLgaJpZM4UJmoS .

dominictarr commented 6 years ago

some of those methods are sync because they are primarily to be called locally, but are sometimes useful to call remotely (or for debugging)... It's in the docs because the manifest is generated from the docs (that goes way back... I think i suggested that in part to try and get paul to write docs (successfully))