Skip to content

Commit 8bb9afa

Browse files
committed
core: improve rpc protocol
- Ensure that pending requests are properly rejected when the underlying service channel is closed. - Remove obsolete id-property from `NotificationMessage`s. Ids are only required fro matching request-response pairs. - Rename `JsonRpcProxyFactory` and related types to `RpcProxyFactory` (Rpc*). The naming scheme was a remainder of the old vscode jsonr-rpc based protocol. By simply using the `Rpc` suffix the class names are less misleading and protocol agnostic. - Keep deprecated declarations of the old `JsonRpc*` namespace. The components are heavily used by adopters so we should maintain this deprecated symbols for a while to enable graceful migration without hard API breaks. - Use a deferred in the `RpcProxyFactory` for protocol initialization Fixes #12581
1 parent c1a2b7b commit 8bb9afa

File tree

3 files changed

+107
-63
lines changed

3 files changed

+107
-63
lines changed

packages/core/src/common/message-rpc/rpc-message-encoder.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ export interface RequestMessage {
5151

5252
export interface NotificationMessage {
5353
type: RpcMessageType.Notification;
54-
id: number;
5554
method: string;
5655
args: any[];
5756
}
@@ -111,7 +110,7 @@ export interface RpcMessageDecoder {
111110
export interface RpcMessageEncoder {
112111
cancel(buf: WriteBuffer, requestId: number): void;
113112

114-
notification(buf: WriteBuffer, requestId: number, method: string, args: any[]): void
113+
notification(buf: WriteBuffer, method: string, args: any[]): void
115114

116115
request(buf: WriteBuffer, requestId: number, method: string, args: any[]): void
117116

@@ -130,8 +129,8 @@ export class MsgPackMessageEncoder implements RpcMessageEncoder {
130129
cancel(buf: WriteBuffer, requestId: number): void {
131130
this.encode<CancelMessage>(buf, { type: RpcMessageType.Cancel, id: requestId });
132131
}
133-
notification(buf: WriteBuffer, requestId: number, method: string, args: any[]): void {
134-
this.encode<NotificationMessage>(buf, { type: RpcMessageType.Notification, id: requestId, method, args });
132+
notification(buf: WriteBuffer, method: string, args: any[]): void {
133+
this.encode<NotificationMessage>(buf, { type: RpcMessageType.Notification, method, args });
135134
}
136135
request(buf: WriteBuffer, requestId: number, method: string, args: any[]): void {
137136
this.encode<RequestMessage>(buf, { type: RpcMessageType.Request, id: requestId, method, args });

packages/core/src/common/message-rpc/rpc-protocol.ts

+9-5
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,11 @@ export class RpcProtocol {
7777
this.encoder = options.encoder ?? new MsgPackMessageEncoder();
7878
this.decoder = options.decoder ?? new MsgPackMessageDecoder();
7979
this.toDispose.push(this.onNotificationEmitter);
80-
channel.onClose(() => this.toDispose.dispose());
80+
channel.onClose(event => {
81+
this.pendingRequests.forEach(pending => pending.reject(new Error(event.reason)));
82+
this.pendingRequests.clear();
83+
this.toDispose.dispose();
84+
});
8185
this.toDispose.push(channel.onMessage(readBuffer => this.handleMessage(this.decoder.parse(readBuffer()))));
8286
this.mode = options.mode ?? 'default';
8387

@@ -98,7 +102,7 @@ export class RpcProtocol {
98102
return;
99103
}
100104
case RpcMessageType.Notification: {
101-
this.handleNotify(message.id, message.method, message.args);
105+
this.handleNotify(message.method, message.args);
102106
return;
103107
}
104108
}
@@ -116,7 +120,7 @@ export class RpcProtocol {
116120
}
117121
}
118122
// If the message was not handled until here, it is incompatible with the mode.
119-
console.warn(`Received message incompatible with this RPCProtocol's mode '${this.mode}'. Type: ${message.type}. ID: ${message.id}.`);
123+
console.warn(`Received message incompatible with this RPCProtocol's mode '${this.mode}'. Type: ${message.type}`);
120124
}
121125

122126
protected handleReply(id: number, value: any): void {
@@ -179,7 +183,7 @@ export class RpcProtocol {
179183
}
180184

181185
const output = this.channel.getWriteBuffer();
182-
this.encoder.notification(output, this.nextMessageId++, method, args);
186+
this.encoder.notification(output, method, args);
183187
output.commit();
184188
}
185189

@@ -226,7 +230,7 @@ export class RpcProtocol {
226230
}
227231
}
228232

229-
protected async handleNotify(id: number, method: string, args: any[]): Promise<void> {
233+
protected async handleNotify(method: string, args: any[]): Promise<void> {
230234
if (this.toDispose.disposed) {
231235
return;
232236
}

packages/core/src/common/messaging/proxy-factory.ts

+95-54
Original file line numberDiff line numberDiff line change
@@ -23,28 +23,30 @@ import { Emitter, Event } from '../event';
2323
import { Channel } from '../message-rpc/channel';
2424
import { RequestHandler, RpcProtocol } from '../message-rpc/rpc-protocol';
2525
import { ConnectionHandler } from './handler';
26+
import { Deferred } from '../promise-util';
2627

27-
export type JsonRpcServer<Client> = Disposable & {
28+
export type RpcServer<Client> = Disposable & {
2829
/**
2930
* If this server is a proxy to a remote server then
3031
* a client is used as a local object
31-
* to handle JSON-RPC messages from the remote server.
32+
* to handle RPC messages from the remote server.
3233
*/
3334
setClient(client: Client | undefined): void;
3435
getClient?(): Client | undefined;
3536
};
3637

37-
export interface JsonRpcConnectionEventEmitter {
38+
export interface RpcConnectionEventEmitter {
3839
readonly onDidOpenConnection: Event<void>;
3940
readonly onDidCloseConnection: Event<void>;
4041
}
41-
export type JsonRpcProxy<T> = T & JsonRpcConnectionEventEmitter;
4242

43-
export class JsonRpcConnectionHandler<T extends object> implements ConnectionHandler {
43+
export type RpcProxy<T> = T & RpcConnectionEventEmitter;
44+
45+
export class RpcConnectionHandler<T extends object> implements ConnectionHandler {
4446
constructor(
4547
readonly path: string,
46-
readonly targetFactory: (proxy: JsonRpcProxy<T>) => any,
47-
readonly factoryConstructor: new () => JsonRpcProxyFactory<T> = JsonRpcProxyFactory
48+
readonly targetFactory: (proxy: RpcProxy<T>) => any,
49+
readonly factoryConstructor: new () => RpcProxyFactory<T> = RpcProxyFactory
4850
) { }
4951

5052
onConnection(connection: Channel): void {
@@ -54,20 +56,30 @@ export class JsonRpcConnectionHandler<T extends object> implements ConnectionHan
5456
factory.listen(connection);
5557
}
5658
}
59+
5760
/**
58-
* Factory for creating a new {@link RpcConnection} for a given chanel and {@link RequestHandler}.
61+
* Factory for creating a new {@link RpcProtocol} for a given chanel and {@link RequestHandler}.
5962
*/
60-
export type RpcConnectionFactory = (channel: Channel, requestHandler: RequestHandler) => RpcProtocol;
63+
export type RpcProtocolFactory = (channel: Channel, requestHandler: RequestHandler) => RpcProtocol;
6164

62-
const defaultRPCConnectionFactory: RpcConnectionFactory = (channel, requestHandler) => new RpcProtocol(channel, requestHandler);
65+
const defaultRpcProtocolFactory: RpcProtocolFactory = (channel, requestHandler) => new RpcProtocol(channel, requestHandler);
66+
67+
export interface RpcProxyFactoryOptions {
68+
rpcProtocolFactory?: RpcProtocolFactory,
69+
/**
70+
* Used to identify proxy calls that should be sent as notification. If method starts with one of the given
71+
* keys it is treated as a notification
72+
*/
73+
notificationKeys?: string[]
74+
}
6375

6476
/**
65-
* Factory for JSON-RPC proxy objects.
77+
* Factory for RPC proxy objects.
6678
*
67-
* A JSON-RPC proxy exposes the programmatic interface of an object through
68-
* JSON-RPC. This allows remote programs to call methods of this objects by
69-
* sending JSON-RPC requests. This takes place over a bi-directional stream,
70-
* where both ends can expose an object and both can call methods each other's
79+
* A RPC proxy exposes the programmatic interface of an object through
80+
* Theia's RPC protocol. This allows remote programs to call methods of this objects by
81+
* sending RPC requests. This takes place over a bi-directional stream,
82+
* where both ends can expose an object and both can call methods on each other's
7183
* exposed object.
7284
*
7385
* For example, assuming we have an object of the following type on one end:
@@ -76,87 +88,87 @@ const defaultRPCConnectionFactory: RpcConnectionFactory = (channel, requestHandl
7688
* bar(baz: number): number { return baz + 1 }
7789
* }
7890
*
79-
* which we want to expose through a JSON-RPC interface. We would do:
91+
* which we want to expose through a JSON interface. We would do:
8092
*
8193
* let target = new Foo()
82-
* let factory = new JsonRpcProxyFactory<Foo>('/foo', target)
94+
* let factory = new RpcProxyFactory<Foo>('/foo', target)
8395
* factory.onConnection(connection)
8496
*
8597
* The party at the other end of the `connection`, in order to remotely call
8698
* methods on this object would do:
8799
*
88-
* let factory = new JsonRpcProxyFactory<Foo>('/foo')
100+
* let factory = new RpcProxyFactory<Foo>('/foo')
89101
* factory.onConnection(connection)
90102
* let proxy = factory.createProxy();
91103
* let result = proxy.bar(42)
92104
* // result is equal to 43
93105
*
94106
* One the wire, it would look like this:
95-
*
96-
* --> {"jsonrpc": "2.0", "id": 0, "method": "bar", "params": {"baz": 42}}
97-
* <-- {"jsonrpc": "2.0", "id": 0, "result": 43}
107+
* --> { "type":"1", "id": 1, "method": "bar", "args": [42]}
108+
* <-- { "type":"3", "id": 1, "res": 43}
98109
*
99110
* Note that in the code of the caller, we didn't pass a target object to
100-
* JsonRpcProxyFactory, because we don't want/need to expose an object.
111+
* RpcProxyFactory, because we don't want/need to expose an object.
101112
* If we had passed a target object, the other side could've called methods on
102113
* it.
103114
*
104-
* @param <T> - The type of the object to expose to JSON-RPC.
115+
* @param <T> - The type of the object to expose to RPC.
105116
*/
106117

107-
export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
118+
export class RpcProxyFactory<T extends object> implements ProxyHandler<T> {
108119

109120
protected readonly onDidOpenConnectionEmitter = new Emitter<void>();
110121
protected readonly onDidCloseConnectionEmitter = new Emitter<void>();
111122

112-
protected connectionPromiseResolve: (connection: RpcProtocol) => void;
113-
protected connectionPromise: Promise<RpcProtocol>;
123+
protected rpcDeferred = new Deferred<RpcProtocol>();
114124

115125
/**
116-
* Build a new JsonRpcProxyFactory.
126+
* Build a new RpcProxyFactory.
117127
*
118-
* @param target - The object to expose to JSON-RPC methods calls. If this
128+
* @param target - The object to expose to RPC methods calls. If this
119129
* is omitted, the proxy won't be able to handle requests, only send them.
120130
*/
121-
constructor(public target?: any, protected rpcConnectionFactory = defaultRPCConnectionFactory) {
131+
constructor(public target?: any, protected rpcProtocolFactory = defaultRpcProtocolFactory) {
122132
this.waitForConnection();
123133
}
124134

125135
protected waitForConnection(): void {
126-
this.connectionPromise = new Promise(resolve =>
127-
this.connectionPromiseResolve = resolve
128-
);
129-
this.connectionPromise.then(connection => {
130-
connection.channel.onClose(() => {
136+
this.rpcDeferred.promise.then(protocol => {
137+
protocol.channel.onClose(() => {
131138
this.onDidCloseConnectionEmitter.fire(undefined);
132139
// Wait for connection in case the backend reconnects
133140
this.waitForConnection();
134141
});
142+
protocol.channel.onError(err => {
143+
if (this.rpcDeferred.state !== 'resolved') {
144+
this.rpcDeferred.reject(err);
145+
}
146+
});
135147
this.onDidOpenConnectionEmitter.fire(undefined);
136148
});
137149
}
138150

139151
/**
140-
* Connect a MessageConnection to the factory.
152+
* Connect a {@link Channel} to the factory by creating an {@link RpcProtocol} on top of it.
141153
*
142-
* This connection will be used to send/receive JSON-RPC requests and
154+
* This protocol will be used to send/receive RPC requests and
143155
* response.
144156
*/
145157
listen(channel: Channel): void {
146-
const connection = this.rpcConnectionFactory(channel, (meth, args) => this.onRequest(meth, ...args));
147-
connection.onNotification(event => this.onNotification(event.method, ...event.args));
158+
const protocol = this.rpcProtocolFactory(channel, (meth, args) => this.onRequest(meth, ...args));
159+
protocol.onNotification(event => this.onNotification(event.method, ...event.args));
148160

149-
this.connectionPromiseResolve(connection);
161+
this.rpcDeferred.resolve(protocol);
150162
}
151163

152164
/**
153-
* Process an incoming JSON-RPC method call.
165+
* Process an incoming RPC method call.
154166
*
155-
* onRequest is called when the JSON-RPC connection received a method call
156-
* request. It calls the corresponding method on [[target]].
167+
* onRequest is called when the RPC connection received a method call
168+
* request. It calls the corresponding method on [[target]].
157169
*
158170
* The return value is a Promise object that is resolved with the return
159-
* value of the method call, if it is successful. The promise is rejected
171+
* value of the method call, if it is successful. The promise is rejected
160172
* if the called method does not exist or if it throws.
161173
*
162174
* @returns A promise of the method call completion.
@@ -181,7 +193,7 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
181193
}
182194

183195
/**
184-
* Process an incoming JSON-RPC notification.
196+
* Process an incoming RPC notification.
185197
*
186198
* Same as [[onRequest]], but called on incoming notifications rather than
187199
* methods calls.
@@ -194,37 +206,37 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
194206

195207
/**
196208
* Create a Proxy exposing the interface of an object of type T. This Proxy
197-
* can be used to do JSON-RPC method calls on the remote target object as
209+
* can be used to do RPC method calls on the remote target object as
198210
* if it was local.
199211
*
200-
* If `T` implements `JsonRpcServer` then a client is used as a target object for a remote target object.
212+
* If `T` implements `RpcServer` then a client is used as a target object for a remote target object.
201213
*/
202-
createProxy(): JsonRpcProxy<T> {
214+
createProxy(): RpcProxy<T> {
203215
const result = new Proxy<T>(this as any, this);
204216
return result as any;
205217
}
206218

207219
/**
208-
* Get a callable object that executes a JSON-RPC method call.
220+
* Get a callable object that executes a RPC method call.
209221
*
210222
* Getting a property on the Proxy object returns a callable that, when
211-
* called, executes a JSON-RPC call. The name of the property defines the
223+
* called, executes a RPC call. The name of the property defines the
212224
* method to be called. The callable takes a variable number of arguments,
213-
* which are passed in the JSON-RPC method call.
225+
* which are passed in the RPC method call.
214226
*
215227
* For example, if you have a Proxy object:
216228
*
217-
* let fooProxyFactory = JsonRpcProxyFactory<Foo>('/foo')
229+
* let fooProxyFactory = RpcProxyFactory<Foo>('/foo')
218230
* let fooProxy = fooProxyFactory.createProxy()
219231
*
220232
* accessing `fooProxy.bar` will return a callable that, when called,
221-
* executes a JSON-RPC method call to method `bar`. Therefore, doing
233+
* executes a RPC method call to method `bar`. Therefore, doing
222234
* `fooProxy.bar()` will call the `bar` method on the remote Foo object.
223235
*
224236
* @param target - unused.
225237
* @param p - The property accessed on the Proxy object.
226238
* @param receiver - unused.
227-
* @returns A callable that executes the JSON-RPC call.
239+
* @returns A callable that executes the RPC call.
228240
*/
229241
get(target: T, p: PropertyKey, receiver: any): any {
230242
if (p === 'setClient') {
@@ -249,7 +261,7 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
249261
return (...args: any[]) => {
250262
const method = p.toString();
251263
const capturedError = new Error(`Request '${method}' failed`);
252-
return this.connectionPromise.then(connection =>
264+
return this.rpcDeferred.promise.then(connection =>
253265
new Promise<void>((resolve, reject) => {
254266
try {
255267
if (isNotify) {
@@ -308,3 +320,32 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
308320

309321
}
310322

323+
/**
324+
* @deprecated since 1.39.0 use `RpcConnectionEventEmitter` instead
325+
*/
326+
export type JsonRpcConnectionEventEmitter = RpcConnectionEventEmitter;
327+
328+
/**
329+
* @deprecated since 1.39.0 use `RpcServer` instead
330+
*/
331+
export type JsonRpcServer<Client> = RpcServer<Client>;
332+
333+
/**
334+
* @deprecated since 1.39.0 use `RpcProxy` instead
335+
*/
336+
export type JsonRpcProxy<T> = RpcProxy<T>;
337+
338+
/**
339+
* @deprecated since 1.39.0 use `RpcConnectionHandler` instead
340+
*/
341+
export class JsonRpcConnectionHandler<T extends object> extends RpcConnectionHandler<T> {
342+
343+
}
344+
345+
/**
346+
* @deprecated since 1.39.0 use `JsonRpcProxyFactory` instead
347+
*/
348+
export class JsonRpcProxyFactory<T extends object> extends RpcProxyFactory<T> {
349+
350+
}
351+

0 commit comments

Comments
 (0)