Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EventEmitterTransport not parsing response either throwing error #188

Closed
jamesblasco opened this issue Jul 16, 2020 · 10 comments · Fixed by #191
Closed

EventEmitterTransport not parsing response either throwing error #188

jamesblasco opened this issue Jul 16, 2020 · 10 comments · Fixed by #191
Labels
bug Something isn't working released
Projects

Comments

@jamesblasco
Copy link

jamesblasco commented Jul 16, 2020

Describe the bug
I am trying to use EventEmitterTransport but I can't manage to make it work.

 this.transport = new rpc.EventEmitterTransport(this, inputUri, outputUri);
 this.client = new rpc.Client(new rpc.RequestManager([this.transport]));
 this.emit(inputUri, '{"jsonrpc":"2.0","result":0,"id":"1"}');

This is not working for me, but it is neither throwing an error.
I am not sure how to continue

For the output, I am doing this and it works correctly

 this.on(outputUri, (data) => {
       let request = JSON.stringify(data);
       console.log(`Request: ${request}`, data);
 });
@shanejonas
Copy link
Member

shanejonas commented Jul 16, 2020

hey @jamesblasco heres a working example:

import { EventEmitter } from "events";
import { RequestManager, EventEmitterTransport, Client } from "@open-rpc/client-js";

const reqUri = "chan1";
const resUri = "chan2";

const emitter = new EventEmitter();
const transport = new EventEmitterTransport(emitter, reqUri, reqUri);
const requestManager = new RequestManager([transport]);
const client = new Client(requestManager);

// event emitter server code
emitter.on(resUri, (jsonrpcRequest) => {
  const res = {
    jsonrpc: "2.0",
    result: "potato",
    id: jsonrpcRequest.id,
  };
  emitter.emit(reqUri, JSON.stringify(res));
});

const main = async () => {
  const result = await client.request("addition", [2, 2]);
  console.log(result);
}

main().then(() => {
  console.log("DONE");
});

@shanejonas
Copy link
Member

i think its confusing because the input/output channels are swapped and we should file a ticket on it

@jamesblasco
Copy link
Author

jamesblasco commented Jul 16, 2020

Thanks for the example!

Could this line

emitter.emit(chan1, JSON.stringify(res));

be replaced by

emitter.emit(chan1, '{"jsonrpc":"2.0","result":"potato","id":"${jsonrpcRequest.id}"}');"

no?

My problem is when trying to emit notifications or responses from the server-side. In my case, server-side is stdin and stdout from a child process in node.js and the response is already parsed

Here is the full code I am using
import * as rpc from "@open-rpc/client-js";
import * as ev from "events";

const inputUri = 'redeable';
const outputUri = 'writable';

export class RcpService extends ev.EventEmitter {
    private redeable: NodeJS.ReadableStream;
    private writable: NodeJS.WritableStream;
    private client: rpc.Client;

    private notificationListeners: Array<(data: any) => void> = [];

    constructor(redeable: NodeJS.ReadableStream, writable: NodeJS.WritableStream) {
        super();
        this.redeable = redeable; 
        this.writable = writable; 

        let transport = new rpc.EventEmitterTransport(this, inputUri, outputUri);
        this.client = new rpc.Client(new rpc.RequestManager([transport]));
        try {

            this.redeable.on('data', this.emitRedeable);

            this.on(outputUri, (data) => {
                let request = JSON.stringify(data);
                console.log(`Output: ${request}`, data);
                this.writable.write(request + '\n'); // Send the output correctly to server
            });

            this.client.onNotification((data: any) => {
                console.log(`Notification: ${data}`); // Never called
                this.notificationListeners.forEach((callback) => callback(data));
            });
            this.client.onError((data: rpc.JSONRPCError) => {
                console.log(`Error: ${data}`); // Never called
            });

            this.client.notify('hey', undefined);
            this.client.request("count", undefined).then((result: any) => {
                console.log('count result: ', result); //Never called
            }).catch((error: any) => {
                console.log('error result: ', error); //Never called

            });

        } catch (e) {
            console.log(e); // never called
        }

    }

    onNotification(callback: (data: any) => void) {
        this.notificationListeners.push(callback);
    }

    private emitRedeable(data: any) {
        console.log(`Input: ${data}`); // Receives from server a correct rcp-json respone string
        //  Here it receives from the child process the response with a valid string format {"jsonrpc":"2.0","result":0,"id":"1"}
        this.emit(inputUri, data); // nothing happens
    }

    dispose() {
        this.removeAllListeners();
    }

}

It is weird because it can't make it to catch an error

@shanejonas
Copy link
Member

shanejonas commented Jul 16, 2020

I think its confusing because the input/output channels are swapped and we should file a ticket on it
#188 (comment)

i pointed out before there is a bug where the input/output channels are swapped so:

this.emit(inputUri, data); // nothing happens

because they are swapped, you are firing the events on the other channel, I just opened a ticket to fix it here: #190

the only reason my example kind of works is that:

my example uses client.request()

which then calls this:

this.connection.emit(this.resUri, parsedData);

firing the event on the resUri instead of reqUri

so with my example server i swapped the bindings for reqUri/resUri.

// event emitter server code
emitter.on(resUri, (jsonrpcRequest) => {
  const res = {
    jsonrpc: "2.0",
    result: "potato",
    id: jsonrpcRequest.id,
  };
  emitter.emit(reqUri, JSON.stringify(res));
});

this all has helped in finding the real bug lol

I have a fix for this coming here #191

@shanejonas shanejonas added the bug Something isn't working label Jul 17, 2020
@shanejonas shanejonas added this to To do in OpenRPC via automation Jul 17, 2020
shanejonas added a commit that referenced this issue Jul 17, 2020
OpenRPC automation moved this from To do to Done Jul 17, 2020
openrpc-bastion added a commit that referenced this issue Jul 17, 2020
## [1.3.2](1.3.1...1.3.2) (2020-07-17)

### Bug Fixes

* correct order of req/res uri for event emitter ([0e5ca00](0e5ca00)), closes [#190](#190) [#188](#188)
@openrpc-bastion
Copy link
Member

🎉 This issue has been resolved in version 1.3.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@shanejonas
Copy link
Member

@jamesblasco fixed up the event emitter being swapped. i think your example should work now

@shanejonas
Copy link
Member

shanejonas commented Jul 17, 2020

working (correct) example now with 1.3.2 released:

import { EventEmitter } from "events";
import { RequestManager, EventEmitterTransport, Client } from "@open-rpc/client-js";

const reqUri = "chan1";
const resUri = "chan2";

const emitter = new EventEmitter();
const transport = new EventEmitterTransport(emitter, reqUri, resUri);
const requestManager = new RequestManager([transport]);
const client = new Client(requestManager);

// event emitter server code
emitter.on(reqUri, (jsonrpcRequest) => {
  const res = {
    jsonrpc: "2.0",
    result: "potato",
    id: jsonrpcRequest.id,
  };
  emitter.emit(resUri, JSON.stringify(res));
});

const main = async () => {
  const result = await client.request("addition", [2, 2]);
  console.log(result);
}

main().then(() => {
  console.log("DONE");
});

@jamesblasco
Copy link
Author

Thank you so much! Now It works great excepts the notifications, I created a new issue about it #194

@shanejonas
Copy link
Member

@jamesblasco just an FYI the .request() interface is changing slightly in 2.0.0 after this PR gets in: #186

@jamesblasco
Copy link
Author

Thank you for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
OpenRPC
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants