Only instanciate multiserver plugins that are actually needed #57
Conversation
…he remote address we are trying to connect to
|
@arj03 @mixmix @christianbundy any thoughts? |
|
Looks good to me. This seems like a good improvement overall. |
|
Agree, nice work.
I don't know this part of the stack so well. My wisdom would be about how
we test this well enough.
OR just merge it and let's see (it will probably explode loudly right?)
…On Thu, 5 Aug 2021, 18:23 Anders Rune Jensen, ***@***.***> wrote:
Looks good to me. This seems like a good improvement overall.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#57 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAUK3HWFH2ATFQXWYOLROZDT3IU5HANCNFSM5A5KTYQQ>
.
|
|
@mixmix Right, there's no test that actually tries the different ways the client can connect to the server. I tried to write one, but failed miserable after way to many hours ... I just can't get one of these two options to work: a) an ssb-server instance that listens on all legal combinations of transport/transforms a) fails because multiserver cannot have more than one unix incoming at a time (but unix/shs and unix/noauth are both needed in the test), because they end up having the same filesystem path (ssb.config.path is reused here) b) fails because, for some reason I don't understand, even two distinct instances of ssb-server, that are started in sequence and whose lifespans do not overlap, cannot each create a unix socket (in this scenario the paths even are different) And finally: I just can't get the onion stuff to work. Could you help out with the test? @cryptix @arj03 |
|
Alternative idea that's not a breaking change. Have an option which enables this mode of yours. And if it's not enabled it behaves as it currently does That way you can get this merged and we can all stress test it? |
Hey!
Since multiserver 3.7.0 was published I ran into an issue using ssb-client from within a browser context. ssb-client is instanciating multiserver's unix-sockets plugin regardless of whether it is needed to connect to the given remote or not. This used to be fine until multiserver 3.7.0, where, during initialisation of that plugin, a fs function is called that is not replaced by browserify's brfs (mkdtmpSync). While this could be fixed in multiserver, I decided to atteck the root problem instead.
This PR builds a custom Multiserver instance that only instanciates the plugins that are mentioned on the remote address that we want to connect to. It still only allows for the transport/protocol combinations that were previously hardcoded.
What do you think?