Added swarm support in embarkjs, isAvailable for messages/storage, swarm/ipfs checks #390

emizzle · 2018-04-30T06:12:58Z

added EmbarkJS.Storage.isAvailable and EmbarkJS.Messages.isAvailable() and underlying provider functions for Whisper, Orbit, IPFS, and Swarm so service availability can be checked from the DApp (@richard-ramos)
Finished swarm implementation in embarkjs plus cleanup
updated test app storage config to swarm to show swarm config option
added eth-lib to parse result of swarm text
added protocol to storage config (defaulting to "http")
selectively starts storage service depending on which one is configured in the storage config
run service check for ipfs/swarm prior to uploaded
added swarm methods for embarkjs
removed unneeded provider property.
add 'swarm' as a provider in the storage.config
update method for swarm service check

Adding swarm to embarkjs. WIP. Add 'auto' setting for geth CORS and websockets origin * 'auto' now supported for `rpcCorsDomain` and `wsOrigins` in the blockchain config. * 'auto' set to the default value in blockchain config for test and demo apps. test add config and contract and add test addFileToPipeline test and registerBeforeDeploy with new arg add more registers but generation one fails in run WIP commit Undo changes to test config. Merge pull request #381 from embark-framework/features/cors-auto Add 'auto' setting for geth CORS and websockets origin fix a bug where upload cmd used plugin name don't error if it's an empty dapp with no contracts yet Merge pull request #383 from embark-framework/no_contracts don't error if it's an empty dapp with no contracts yet remove duplicated entry force zepplein version for travis Merge pull request #384 from embark-framework/chores/test-allpligin-apis Small fixes for plugin APIs intercept logs in the app itself - stopgap fix Merge pull request #385 from embark-framework/console_logs_fix intercept logs in the app itself - stopgap fix * removed unneeded provider property. * add 'swarm' as a provider in the storage.config * update method for swarm service check Merge branch 'develop' into features/add-swarm-to-embarkjs More work to add swarm to embarkjs * added eth-lib to parse result of swarm text * changed "currentStorage" and "currentMessages" to "currentProvider" for consistency. * added protocol to storage config * selectively starts storage service depending on which one is configured in the storage config * run service check for ipfs/swarm prior to uploaded * added swarm methods for embarkjs Updated code based on code review check if testrpc is installed and warn if not Merge pull request #386 from embark-framework/bug_fix/test-rpc-not-installed check if testrpc is installed and warn if not Removed timeout Removed spacer Merge pull request #382 from embark-framework/react-demo Updating embark demo to use react instead of jquery fix on contract add Merge pull request #387 from embark-framework/bug_fix/new-contract-in-empty-dapp Fix adding a contract redeploy with right config on config change fix tests reset watchers after build to make sure files remain watch Merge pull request #389 from embark-framework/bug_fix/file-changes-not-watched Fix files not being watched Merge pull request #388 from embark-framework/bug_fix/changing-contract-config Redeploy with right config on config change Added swarm support in embarkjs and isAvailable for messages/storage * reverted currentProvider back to currentStorage and currentMessages * added `EmbarkJS.Storage.isAvailable` and `EmbarkJS.Messages.isAvailable()` and underlying provider functions for Whisper, Orbit, IPFS, and Swarm * Finished swarm implementation in embarkjs plus cleanup * updated test app storage config to swarm to show swarm config option Merge branch 'develop' into features/add-swarm-to-embarkjs

…rm-to-embarkjs' of https://github.com/embark-framework/embark into features/add-swarm-to-embarkjs

emizzle · 2018-04-30T13:28:24Z

on the test app, run ../../bin/embark upload ipfs or ../../bin/embark upload swarm causes the following error:

/Users/emizzle/Code/__Github/embark/lib/core/engine.js:149
          self.watch.restart(); // Necessary because changing a file while it is writing can stop it from being watched
                     ^

TypeError: Cannot read property 'restart' of undefined
    at /Users/emizzle/Code/__Github/embark/lib/core/engine.js:149:22
    at /Users/emizzle/Code/__Github/embark/lib/pipeline/pipeline.js:155:11
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:473:16
    at replenish (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:993:25)
    at iterateeCallback (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:983:17)
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:958:16
    at /Users/emizzle/Code/__Github/embark/lib/pipeline/pipeline.js:150:13
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:1126:9
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:473:16
    at replenish (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:993:25)
    at iterateeCallback (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:983:17)
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:958:16
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:1123:13
    at /Users/emizzle/Code/__Github/embark/lib/pipeline/pipeline.js:180:16
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:473:16
    at replenish (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:993:25)
    at iterateeCallback (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:983:17)
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:958:16
    at /Users/emizzle/Code/__Github/embark/lib/pipeline/pipeline.js:174:9
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:3096:16
    at replenish (/Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:998:17)
    at /Users/emizzle/Code/__Github/embark/node_modules/async/dist/async.js:1002:9

…rm-to-embarkjs' of https://github.com/embark-framework/embark into features/add-swarm-to-embarkjs

…into features/add-swarm-to-embarkjs

jrainville

Mostly the same comment at multiple places about returning to prevent further code execution and also because it's cleaner. Otherwise, it's pretty gucci 👍

jrainville · 2018-04-30T12:33:39Z

lib/index.js

+            return true;
+          }
+        });
+        if(!checkFn || typeof checkFn.fn !== 'function') callback();


Just code cleaning: You could just do:

if(!checkFn || typeof checkFn.fn !== 'function') { return callback(); }

That way, we don't need an else.

jrainville · 2018-04-30T12:35:58Z

lib/index.js

+            if(!serviceCheckResult.status || serviceCheckResult.status === 'off'){
+              callback({message:`Cannot upload: ${platform} node is not running on http://${engine.config.storageConfig.host}:${engine.config.storageConfig.port}.`});
+            } else {
+              callback(null);


Again, you can use a return from the first one to remove the else. But mostly, no need to put null in the callback.

jrainville · 2018-04-30T12:36:14Z

lib/index.js

-          callback();
-        }
+        } 
+        else callback(null);


Same thing as above.

jrainville · 2018-04-30T12:38:49Z

lib/modules/ipfs/embarkjs.js


+__embarkIPFS.isAvailable = function(){
+  return new Promise((resolve) => {
+    if(!this.ipfsConnection) resolve(false);


Don't you need a return here to make sure we don't resolve twice (not sure if it is possible, but at least to prevent anymore code execution)

Ok, I tested and it does only take the first resolve, but it also does continue the code execution, so I think using a return would still be nice.

jrainville · 2018-04-30T14:17:49Z

lib/modules/swarm/embarkjs.js

+  this.connectError = new Error(`Cannot connect to Swarm node on ${this.connectUrl}`);
+  this._getUrl = options.getUrl || `${this.connectUrl}/bzzr:/`;
+
+  var promise = new Promise((resolve, reject) => {


No need for the var. Just return new Promise((resolve, reject) => { ......

jrainville · 2018-04-30T14:18:17Z

lib/modules/swarm/embarkjs.js

+__embarkSwarm.saveText = function(text) {
+  return new Promise((resolve, reject) => {
+    this.isAvailable().then((isAvailable) => {
+      if(!isAvailable) reject(this.connectError);


Again return

jrainville · 2018-04-30T14:18:47Z

lib/modules/swarm/embarkjs.js

+__embarkSwarm.get = function(hash) {
+  return new Promise((resolve, reject) => {
+    this.isAvailable().then((isAvailable) => {
+      if(!isAvailable) reject(this.connectError);


Again return (sorry for copy-pasta, just want you to know all the spots 🤷‍♂️ )

jrainville · 2018-04-30T14:19:31Z

lib/modules/swarm/embarkjs.js

+    reader.onloadend = (event) => {
+      var fileContent = new Uint8Array(event.target.result);
+      this.isAvailable().then((isAvailable) => {
+        if(!isAvailable) reject(this.connectError);


Again return

jrainville · 2018-04-30T14:22:15Z

lib/modules/whisper/js/embarkjs.js


+__embarkWhisperNewWeb3.isAvailable = function(){
+  return new Promise((resolve, reject) => {
+    if(!this.web3.shh) resolve(false);


Again return

jrainville · 2018-04-30T14:23:10Z

lib/modules/whisper/js/embarkjs_old_web3.js


+__embarkWhisperOld.isAvailable = function(){
+  return new Promise((resolve, reject) => {
+    if(!this.web3) resolve(false);


Again return

iurimatias · 2018-05-01T12:35:02Z

@jrainville could you add the missing early returns please and then merge? @emizzle is currently traveling

jrainville

As far as I'm concerned, the code is good. I tested in the demo app and it worked ok. Sometimes, the image returned a 0 byte file, but could be swarm-gateways. However, upload swarm returns with a dir_hash of false (both in the demo app and test app) and I'm not sure why that is. @emizzle is there another step to configure?

emizzle · 2018-05-02T20:24:33Z

Returning 0 byte files and returning a hash that 404's on the network are exactly the issues I was having with swarm. What does your storage.json look like @jrainville?

jrainville · 2018-05-02T20:30:25Z

I went back to IPFS after testing, but I had what you put in the test_app:

{
  "default": {
    "enabled": true,
    "available_providers": ["ipfs", "swarm"],
    "ipfs_bin": "ipfs",
    "provider": "ipfs",
    "host": "localhost",
    "port": 5001
  },
  "development": {
    "enabled": true,
    "provider": "swarm",
    "host": "swarm-gateways.net",
    "port": 80,
    "getUrl": "http://swarm-gateways.net/bzzr:/"
  },
  "livenet": {
    "enabled": true,
    "provider": "ipfs",
    "host": "ipfs.infura.io",
    "protocol": "https",
    "port": false,
    "getUrl": "https://ipfs.infura.io/ipfs/"
  }
}

emizzle · 2018-05-02T21:35:39Z

The config looks good. I just feel like Swarm is strangely unreliable. Mostly on localhost, getting a resource seemed to return a 200 but with 0 bytes. While swarm-gateways.net seems to 404 most often (and sometimes the resource will eventually become available). And other times, I'll get a CORS error when trying to get a resource. It's definitely flaky.

In saying that, I have never hit an issue with not getting a hash.

And even weirder, I swear sometimes it would seem to work better when running a local node, but using swarm-gateways.net. But that would never be 100% reliable either. Witch craft.

emizzle added 2 commits April 30, 2018 15:56

Merge branch 'develop' into features/add-swarm-to-embarkjs

b29965b

emizzle changed the title ~~Added swarm support in embarkjs and isAvailable for messages/storage~~ Added swarm support in embarkj, isAvailable for messages/storage, swarm/ipfs checks Apr 30, 2018

emizzle requested review from VoR0220, iurimatias, jrainville and richard-ramos April 30, 2018 06:18

emizzle changed the title ~~Added swarm support in embarkj, isAvailable for messages/storage, swarm/ipfs checks~~ Added swarm support in embarkjs, isAvailable for messages/storage, swarm/ipfs checks Apr 30, 2018

emizzle added 2 commits April 30, 2018 22:07

Fixes to make lint and tests happy

72cce7d

Merge branches 'features/add-swarm-to-embarkjs' and 'features/add-swa…

8c59118

…rm-to-embarkjs' of https://github.com/embark-framework/embark into features/add-swarm-to-embarkjs

emizzle mentioned this pull request Apr 30, 2018

Updating embark demo to use react instead of jquery #382

Merged

emizzle added 3 commits May 1, 2018 00:03

include fix for bug

6ef78ff

Merge branches 'features/add-swarm-to-embarkjs' and 'features/add-swa…

b93b3b4

…rm-to-embarkjs' of https://github.com/embark-framework/embark into features/add-swarm-to-embarkjs

Merge remote-tracking branch 'origin/features/add-swarm-to-embarkjs' …

8770e1e

…into features/add-swarm-to-embarkjs

jrainville reviewed Apr 30, 2018

View reviewed changes

jrainville added 2 commits May 1, 2018 09:38

add early returns and lint

2ad8526

more linting and add condition in case of no hash

3bf5093

jrainville approved these changes May 1, 2018

View reviewed changes

iurimatias merged commit 0cffff0 into develop May 1, 2018

jrainville deleted the features/add-swarm-to-embarkjs branch May 3, 2018 16:14

Added swarm support in embarkjs, isAvailable for messages/storage, swarm/ipfs checks #390

Added swarm support in embarkjs, isAvailable for messages/storage, swarm/ipfs checks #390

Conversation

emizzle commented Apr 30, 2018

Uh oh!

emizzle commented Apr 30, 2018

Uh oh!

jrainville left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iurimatias commented May 1, 2018

Uh oh!

jrainville left a comment

Choose a reason for hiding this comment

Uh oh!

emizzle commented May 2, 2018

Uh oh!

jrainville commented May 2, 2018

Uh oh!

emizzle commented May 2, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants