-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Decouple HTTP Servers from cli/commands/daemon #1950
Conversation
In the past API was exposed via HTTP Server only when jsipfs daemon was run from the commandline, so src/http/index.js was also responsible for orchestration that is not related to HTTP itself. This refactor moves code that is not related to HTTP Servers into standalone-daemon.js, which is easier to reason about, and unlocks use of HttpApi in contexts other than commandline jsipfs daemon, such as Firefox with libdweb or Chromium-based web browser with chrome.sockets APIs. Refs. ipfs/ipfs-companion#664 License: MIT Signed-off-by: Marcin Rataj <[email protected]>
This changes behavior in web browser. Instead of printing to console.log, it uses proper debug-based logger. Old behavior in terminal (when run via `jsipfs daemon`) does not change. License: MIT Signed-off-by: Marcin Rataj <[email protected]>
29a91ce
to
0b9076d
Compare
Ok, should be ready, let's review it. |
This replaces durect use of HttpApi with StandaloneDaemon, restoring all existing tests to operational state. License: MIT Signed-off-by: Marcin Rataj <[email protected]>
e326540
to
7c32da8
Compare
src/cli/standalone-daemon.js
Outdated
reject(err) | ||
}) | ||
ipfs.once('start', resolve) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pair program and work on the test together?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alanshaw I'd prefer to merge this PR as-is and improve it later.
@pkafei Sure! I am booked today, but feel free to grab time slot next week: https://calendly.com/pl-lidel/ :)
src/cli/daemon.js
Outdated
}) | ||
this._httpApi._apiServers.forEach(apiServer => { | ||
ipfs._print('Web UI available at %s', toUri(apiServer.info.ma) + '/webui') | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant move to src/cli/commands/daemon.js
- where the rest of the logging already is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, done in 1e82102
License: MIT Signed-off-by: Marcin Rataj <[email protected]>
a2a307d
to
1e82102
Compare
}) | ||
daemon._httpApi._apiServers.forEach(apiServer => { | ||
print(`Web UI available at ${toUri(apiServer.info.ma)}/webui`) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
This applies cherry-picked patches from: ipfs/js-ipfs#1989 ipfs/js-ipfs#1950 and solves stream issues on page refresh. Content-type sniffing is now done over a meaningful amount of bytes instead of arbitrary number.
Thanks @lidel ❤️ |
Summary
This PR decouples HTTP servers from code
jsipfs daemon
.Apart from cleanup, we need this to enable HTTP server in browser context with
chrome.sockets
.There are no functional changes. I just moved non-HTTP code to
src/cli/standalone-daemon.js
Background
In the past API was exposed via HTTP Server only when jsipfs daemon was run from the commandline, so src/http/index.js was also responsible for orchestration that is not related to HTTP itself. The coupling was quite bad:
HttpApi
basically decided on libp2p configuration: it enabled WebRTC if available, TCP transport, MulticastDNS etc.Changes
This refactor moves code that is not related to HTTP Servers into
standalone-daemon.js
, which is easier to reason about, and unlocks use ofHttpApi
in contexts other than CLIjsipfs daemon
, such as Firefox with libdweb or Chromium-based web browser withchrome.sockets
APIsWith this PR, HttpApi is not only decoupled from Standalone Daemon, but can also be manually started/stopped in browser context.
I believe only ipfs-companion will be using this for now, but it is a fairly generic code.
Someone else should be able to use this in Chrome App environment:
Start
Stop
Checklist
- DONE: does not block this PR (it was due to Node-specific check in Hapi, will be addressed upstream)
Refs.
ipfs/ipfs-companion#664, #1855, #1820