bigstepinc / jsonrpc-bidirectional

Bidirectional RPC over WebSocket, Worker, WebRTC and HTTP with extensive plugin support.
MIT License
110 stars 22 forks source link

cannot make RPC call through WebSocket transport #41

Closed yingfangdu closed 6 years ago

yingfangdu commented 6 years ago

I installed the version 6.5 and use this in the electron-quick-start sample project. While I cannot make any RPC call as the OutgoingRequest.isMethodCalled is always true.

const JSONRPC  = require("jsonrpc-bidirectional");
const WebSocket = require("ws");

async function loadThroughJsonRPC() {
    const defaultAPIEndPoint = 'ws://localhost:8080'
    const ws = new WebSocket(defaultAPIEndPoint);
    await new Promise((resolve, reject) => {
      ws.on("open", resolve);
      ws.on("error", reject);
    });

    const rpcClient = new JSONRPC.Client(defaultAPIEndPoint);
    const webSocketTransport = new JSONRPC.Plugins.Client.WebSocketTransport(ws);
    rpcClient.addPlugin(webSocketTransport);

    var request = {
            Name:'AccountService',
            MethodName:'GetAccountCount',
            Parameters:["1"]
          };
    await rpcClient.rpc("CallService", [request]);
}
oxygen commented 6 years ago

Please update your post with the nodejs server part which reproduces the issues you are having, the electron version and your OS.

isMethodCalled is by default false. The WebSocketTransport plugin makes it true.

There is a getter for isMethodCalled in OutgoingRequest. You could debug there and see who sets it to true. My bet would be that it is WebSocketTransport.

yingfangdu commented 6 years ago

thanks for the response, I did not see any error in the console. I am using node V10.0.0.

I also tried with other json rpc like 'jsonrpc-websocket-client', it works correct.

The sample project I use is electron-quick-start

oxygen commented 6 years ago

AllTests.callRPCMethodNonBidirectionalClient() covers the scenario of a simple client with a WS transport making calls to a ws and uws based server and it passes just fine with the jsonrpc-bidirectional server of course. I just ran it on node v10 and it passes. It is already used in an internal company electron app at my company and works there as well (switched it to electron 1.8.6 and ws 5.1.1 just to test with the latest libraries for this issue).

I just tested now with an empty pathname (simple /) in the endpoint URL and it works. Could you change defaultAPIEndPoint to 'ws://localhost:8080/' (add a slash) and see if it resolves the issue?

What WebSocket and JSONRPC servers are you using?

oxygen commented 6 years ago

Fixed normalizePath to correctly handle undefined url.parse().pathname properties resulting for "" paths.

Published v6.6.1

yingfangdu commented 6 years ago

thanks very much for helping me this out.

I tried to fix that with 'ws://localhost:8080/' , it does not solve the error. I paste a screen shot. After I upgrade to v6.6.1, the issue is gone.

bi

yingfangdu commented 6 years ago

OK, I find another issue with me, maybe should I open it as another issue?

I am using this package in a react app. and when i build it with react-scripts build && npm run release command, it gets build errors like this:

E:\Repos\BAE\src\electron\htmlui> npm run build

bae-electron@0.1.0 build E:\Repos\BAE\src\electron\htmlui react-scripts build && npm run release

Creating an optimized production build...Failed to compile.

Failed to minify the code from this file:

    ./node_modules/jsonrpc-bidirectional/src/Exception.js:10

Read more here: http://bit.ly/2tRViJ9 npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! bae-electron@0.1.0 build: react-scripts build && npm run release npm ERR! Exit status 1npm ERR! npm ERR! Failed at the bae-electron@0.1.0 build script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in: npm ERR! C:\Users\yingfand\AppData\Roaming\npm-cache_logs\2018-05-01T21_59_10_077Z-debug.log

yingfangdu commented 6 years ago

At last, I change to use react-scripts@2.0.0-next.66cc7a90 solves my issue.