poelstra / mhub

Simple, flexible message hub using websockets
MIT License
9 stars 7 forks source link

Extracting nodeserver #14

Open rikkertkoppes opened 6 years ago

rikkertkoppes commented 6 years ago

MHub currently already has an exported MClient, which is defined in nodeclient.ts. However, there is no nodeserver equivalent of it.

If one wants to embed a nodejs server into another application, one has to create a hub initialize transports for it, basically a lot of what is defined in mhub-server.ts, which is the cli script.

My proposal is to create a nodeserver.ts, extracting the server relevant bits from mhub-server (like creating the hub, setting up transports and authenticators and adding nodes). This is basically the bottom part of mhub-server.ts

I am happy to work on it and create a pull request for it. Are there any ideas / thoughts you have that I should take into account before working on this?

rikkertkoppes commented 6 years ago

Progress here: https://github.com/rikkertkoppes/mhub/tree/nodeserver

I am currently cleaning up in small steps. Some intermediate goals are:

poelstra commented 6 years ago

Cool! Progress looks good so far!

I'll hopefully have some time tomorrow to properly review the changes.

Small remark: please use tabs for indentation (I thought .editorconfig is setup for that, maybe I made a mistake there).

rikkertkoppes commented 6 years ago

Tabs, wat?

rikkertkoppes commented 6 years ago

Can you elaborate on this bit:

const logLevelName = args.loglevel || config.logging;
if (config.logging) {
    // Convert config.logging to a LogLevel
    const found = Object.keys(LogLevel).some((s) => {
        if (s.toLowerCase() === logLevelName) {
            log.logLevel = (<any>LogLevel)[s] as LogLevel;
            return true;
        }
        return false;
    });
    if (!found) {
        die(`Invalid log level '${logLevelName}', expected one of: ${logLevelNames.join(", ")}`);
    }
} else if (config.verbose === undefined || config.verbose) {
    log.logLevel = LogLevel.Debug;
}

(https://github.com/poelstra/mhub/blob/master/src/mhub-server.ts#L160-L175)

So if there is no logging key in the config, then the level always defaults to Debug if verbose and Info (default in Logger) otherwise, even when a cli argument is given.

Seems the first if should check for logLevelName

poelstra commented 6 years ago

I think you are right, and it should have been if (logLevelName) indeed.

rikkertkoppes commented 6 years ago

I have added loads of commits to the branch, taking baby steps along the way. The main ideas are

Storage and logging remain a bit of a pain, since they are global. Considering you can now create multiple instances of a server, we may want to change that a bit

poelstra commented 6 years ago

Storage is now non-global, logging is still global.

Your PR was merged (as it started to contain useful things I'd also like to have on master), so please create a new PR with any follow-up work as necessary. I'll keep this item open to track overall status.