Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Run tests inside electron Node.js and Browser process as well #697

Closed
harshjv opened this issue Jan 3, 2017 · 8 comments
Closed

Run tests inside electron Node.js and Browser process as well #697

harshjv opened this issue Jan 3, 2017 · 8 comments
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked

Comments

@harshjv
Copy link
Contributor

harshjv commented Jan 3, 2017

When running js-ipfs daemon with Electron via JS API with js-ipfs daemon running in background, js-ipfs in Electron process throws this error every time it encounters a peer (other js-ipfs node).

Configuration

Electron node

"Addresses": {
  API: "/ip4/127.0.0.1/tcp/5003",
  Swarm: [
    "/ip4/0.0.0.0/tcp/4003",
    "/libp2p-webrtc-star/ip4/188.166.203.82/tcp/20000/ws/ipfs/<PEER-ID>"
  ],
  Gateway: "/ip4/0.0.0.0/tcp/8083"
}

js-ipfs node

"Addresses": {
  API: "/ip4/127.0.0.1/tcp/5002",
  Swarm: [
    "/ip4/0.0.0.0/tcp/4002",
    "/libp2p-webrtc-star/ip4/188.166.203.82/tcp/20000/ws/ipfs/<PEER-ID>"
  ],
  Gateway: "/ip4/0.0.0.0/tcp/8082"
}

Error in js-ipfs node in electron process

Uncaught Exception:
TypeError: Cannot read property 'getRandomValues' of undefined
    at Object.exports.randomBytes (/Users/Project/app/node_modules/libp2p-secio/src/support.js:80:38)
    at Object.exports.createProposal (/Users/Project/app/node_modules/libp2p-secio/src/handshake/crypto.js:21:19)
    at propose (/Users/Project/app/node_modules/libp2p-secio/src/handshake/propose.js:18:31)
    at series (/Users/Project/app/node_modules/libp2p-secio/src/handshake/index.js:13:13)
    at /Users/Project/app/node_modules/async/internal/parallel.js:27:9
    at replenish (/Users/Project/app/node_modules/async/internal/eachOfLimit.js:64:17)
    at /Users/Project/app/node_modules/async/internal/eachOfLimit.js:68:9
    at eachOfLimit (/Users/Project/app/node_modules/async/eachOfLimit.js:37:36)
    at /Users/Project/app/node_modules/async/internal/doLimit.js:9:16
    at _parallel (/Users/Project/app/node_modules/async/internal/parallel.js:26:5)
    at series (/Users/Project/app/node_modules/async/series.js:83:26)
    at handshake (/Users/Project/app/node_modules/libp2p-secio/src/handshake/index.js:12:3)
    at encrypt (/Users/Project/app/node_modules/libp2p-secio/src/index.js:36:7)
    at Object.swarm.handle [as handlerFunc] (/Users/Project/app/node_modules/libp2p-swarm/src/connection.js:87:24)
    at matcher (/Users/Project/app/node_modules/multistream-select/src/listener/select-handler.js:40:28)
    at some (/Users/Project/app/node_modules/multistream-select/src/listener/select-handler.js:71:7)

Platform

Electron v1.4.13

@harshjv
Copy link
Contributor Author

harshjv commented Jan 3, 2017

This issue is more complex than I thought. The problem is with dependencies using OpenSSL but Chromium is using some fork of OpenSSL.

I found some related issues with dependencies worth tracking for this issue:

libp2p/js-libp2p-crypto#38
PeculiarVentures/node-webcrypto-ossl#77
electron/electron#1410 (comment)

:/

@daviddias
Copy link
Member

Oh, this is super annoying :( So in essence, js-ipfs in the Node.js process of electron won't work because of incompatible crypto binding, it will work in the Browser process only since it uses WebCrypto directly.

I see that the electron team tagged it as wont fix, offering the alternative that modules using it will have to bundle the native crypto library themselves. This actually doesn't go that far from that we had discussed since we wanted to remove that ugly install experience anyway. The hard problem is really with shipping bundling 'crypto binaries' (what could go wrong there).

node-webcrypto-ossl is actually an optional dependency at the moment and there is a fallback to straight JavaScript, making it slower, but functional. I would argue that is best to remove node-webcrypto-ossl entirely for the moment until there is a good solid solution, removing the frustration for both electron users and windows users. @dignifiedquire am I missing anything that might go against this proposal?

Thank you for the report and debugging @harshjv

@haadcode
Copy link
Member

haadcode commented Jan 9, 2017

I would argue that is best to remove node-webcrypto-ossl entirely for the moment until there is a good solid solution, removing the frustration for both electron users and windows users.

As much I would not like to downgrade the crypto perf, I would agree with @diasdavid's proposal.

@daviddias daviddias added the status/ready Ready to be worked label Jan 11, 2017
@daviddias
Copy link
Member

Thank you @haadcode. Anyone against?

@dignifiedquire
Copy link
Member

Yes I am. As mentioned before we can detect electron and fallback to pure js in the main process, without needing to drop the perf improvements in regular node.js scenarios.

@daviddias
Copy link
Member

Let's try that :)

@daviddias daviddias added kind/bug A bug in existing code (including security flaws) exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue exp/expert Having worked on the specific codebase is important and removed exp/novice Someone with a little familiarity can pick up labels Jan 19, 2017
@daviddias daviddias changed the title TypeError: Cannot read property 'getRandomValues' of undefined [electron] TypeError: Cannot read property 'getRandomValues' of undefined Jan 20, 2017
@daviddias
Copy link
Member

@dignifiedquire also, can we get aegir to also run the tests inside electron for us? Same way as in Node.js, Browser and WebWorker. Ideally, we would run tests inside the browser process and node.js process.

@daviddias daviddias changed the title [electron] TypeError: Cannot read property 'getRandomValues' of undefined [electron] TypeError: Cannot read property 'getRandomValues' of undefined Jan 29, 2017
@daviddias daviddias added status/deferred Conscious decision to pause or backlog and removed status/ready Ready to be worked labels Jan 29, 2017
@daviddias daviddias changed the title [electron] TypeError: Cannot read property 'getRandomValues' of undefined Running tests inside electron Node.js and Browser process Mar 21, 2017
@daviddias daviddias changed the title Running tests inside electron Node.js and Browser process Run tests inside electron Node.js and Browser process as well Mar 21, 2017
@daviddias daviddias added status/ready Ready to be worked and removed status/deferred Conscious decision to pause or backlog labels Apr 5, 2017
@daviddias
Copy link
Member

Created ipfs/aegir#157 in aegir to track this. #843 is the main issue to track supporting electron. Closing this one :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

4 participants