Skip to content

Commit

Permalink
fix(Grpc-Web): Fix compilation failure when a service definition cont…
Browse files Browse the repository at this point in the history
…ains a client streaming call. (#535)

Note this does not implement client streaming, instead the function will throw due to the feature not being implemented. This is a stop-gap until true client streaming support is added.
  • Loading branch information
zakhenry committed Mar 27, 2022
1 parent 36dccb4 commit 0c83892
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 15 deletions.
12 changes: 11 additions & 1 deletion integration/grpc-web/example-test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { DashStateClientImpl } from './example';
import { EMPTY } from 'rxjs';

describe('grpc-web', () => {
it('at least compiles', () => {
Expand All @@ -19,5 +20,14 @@ describe('grpc-web', () => {
const client = new DashStateClientImpl(rpc);
const userSettings = client.UserSettings;
userSettings({});
})
});
it('throws on client streaming call', () => {
const rpc = {
unary: jest.fn(),
invoke: jest.fn(),
};
const client = new DashStateClientImpl(rpc);
const call = () => client.ChangeUserSettingsStream(EMPTY)
expect(call).toThrowError("ts-proto does not yet support client streaming!")
});
});
Binary file modified integration/grpc-web/example.bin
Binary file not shown.
2 changes: 2 additions & 0 deletions integration/grpc-web/example.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package rpx;
service DashState {
rpc UserSettings(Empty) returns (DashUserSettingsState);
rpc ActiveUserSettingsStream(Empty) returns (stream DashUserSettingsState);
// not supported in grpc-web, but should still compile
rpc ChangeUserSettingsStream (stream DashUserSettingsState) returns (stream DashUserSettingsState) {}
}

message DashFlash {
Expand Down
13 changes: 13 additions & 0 deletions integration/grpc-web/example.ts
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,11 @@ export const Empty = {
export interface DashState {
UserSettings(request: DeepPartial<Empty>, metadata?: grpc.Metadata): Promise<DashUserSettingsState>;
ActiveUserSettingsStream(request: DeepPartial<Empty>, metadata?: grpc.Metadata): Observable<DashUserSettingsState>;
/** not supported in grpc-web, but should still compile */
ChangeUserSettingsStream(
request: Observable<DeepPartial<DashUserSettingsState>>,
metadata?: grpc.Metadata
): Observable<DashUserSettingsState>;
}

export class DashStateClientImpl implements DashState {
Expand All @@ -604,6 +609,7 @@ export class DashStateClientImpl implements DashState {
this.rpc = rpc;
this.UserSettings = this.UserSettings.bind(this);
this.ActiveUserSettingsStream = this.ActiveUserSettingsStream.bind(this);
this.ChangeUserSettingsStream = this.ChangeUserSettingsStream.bind(this);
}

UserSettings(request: DeepPartial<Empty>, metadata?: grpc.Metadata): Promise<DashUserSettingsState> {
Expand All @@ -613,6 +619,13 @@ export class DashStateClientImpl implements DashState {
ActiveUserSettingsStream(request: DeepPartial<Empty>, metadata?: grpc.Metadata): Observable<DashUserSettingsState> {
return this.rpc.invoke(DashStateActiveUserSettingsStreamDesc, Empty.fromPartial(request), metadata);
}

ChangeUserSettingsStream(
request: Observable<DeepPartial<DashUserSettingsState>>,
metadata?: grpc.Metadata
): Observable<DashUserSettingsState> {
throw new Error('ts-proto does not yet support client streaming!');
}
}

export const DashStateDesc = {
Expand Down
27 changes: 19 additions & 8 deletions src/generate-grpc-web.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MethodDescriptorProto, FileDescriptorProto, ServiceDescriptorProto } from 'ts-proto-descriptors';
import { requestType, responsePromiseOrObservable, responseType } from './types';
import { rawRequestType, requestType, responsePromiseOrObservable, responseType } from './types';
import { Code, code, imp, joinCode } from 'ts-poet';
import { Context } from './context';
import { assertInstanceOf, FormattedMethodDescriptor, maybePrefixPackage } from './utils';
Expand Down Expand Up @@ -35,33 +35,44 @@ export function generateGrpcClientImpl(
assertInstanceOf(methodDesc, FormattedMethodDescriptor);
chunks.push(code`this.${methodDesc.formattedName} = this.${methodDesc.formattedName}.bind(this);`);
}
chunks.push(code`}\n`);
chunks.push(code`}`);

// Create a method for each FooService method
for (const methodDesc of serviceDesc.method) {
chunks.push(generateRpcMethod(ctx, serviceDesc, methodDesc));
}

chunks.push(code`}`);
return joinCode(chunks, { trim: false });
return joinCode(chunks, { trim: false, on: '\n' });
}

/** Creates the RPC methods that client code actually calls. */
function generateRpcMethod(ctx: Context, serviceDesc: ServiceDescriptorProto, methodDesc: MethodDescriptorProto) {
assertInstanceOf(methodDesc, FormattedMethodDescriptor);
const { options, utils } = ctx;
const inputType = requestType(ctx, methodDesc);
const partialInputType = code`${utils.DeepPartial}<${inputType}>`;
const requestMessage = rawRequestType(ctx, methodDesc);
const inputType = requestType(ctx, methodDesc, true);
const returns = responsePromiseOrObservable(ctx, methodDesc);

if (methodDesc.clientStreaming) {
return code`
${methodDesc.formattedName}(
request: ${inputType},
metadata?: grpc.Metadata,
): ${returns} {
throw new Error('ts-proto does not yet support client streaming!');
}
`;
}

const method = methodDesc.serverStreaming ? 'invoke' : 'unary';
return code`
${methodDesc.formattedName}(
request: ${partialInputType},
request: ${inputType},
metadata?: grpc.Metadata,
): ${returns} {
return this.rpc.${method}(
${methodDescName(serviceDesc, methodDesc)},
${inputType}.fromPartial(request),
${requestMessage}.fromPartial(request),
metadata,
);
}
Expand Down
6 changes: 2 additions & 4 deletions src/generate-services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,10 @@ export function generateService(
params.push(code`ctx: Context`);
}

let inputType = requestType(ctx, methodDesc);
// the grpc-web clients auto-`fromPartial` the input before handing off to grpc-web's
// serde runtime, so it's okay to accept partial results from the client
if (options.outputClientImpl === 'grpc-web') {
inputType = code`${utils.DeepPartial}<${inputType}>`;
}
const partialInput = options.outputClientImpl === 'grpc-web';
const inputType = requestType(ctx, methodDesc, partialInput);
params.push(code`request: ${inputType}`);

// Use metadata as last argument for interface only configuration
Expand Down
4 changes: 3 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ export function generateFile(ctx: Context, fileDesc: FileDescriptorProto): [stri
chunks.push(generateGrpcClientImpl(ctx, fileDesc, serviceDesc));
chunks.push(generateGrpcServiceDesc(fileDesc, serviceDesc));
serviceDesc.method.forEach((method) => {
chunks.push(generateGrpcMethodDesc(ctx, serviceDesc, method));
if (!method.clientStreaming) {
chunks.push(generateGrpcMethodDesc(ctx, serviceDesc, method));
}
if (method.serverStreaming) {
hasServerStreamingMethods = true;
}
Expand Down
7 changes: 6 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,13 @@ export function rawRequestType(ctx: Context, methodDesc: MethodDescriptorProto):
return messageToTypeName(ctx, methodDesc.inputType);
}

export function requestType(ctx: Context, methodDesc: MethodDescriptorProto): Code {
export function requestType(ctx: Context, methodDesc: MethodDescriptorProto, partial: boolean = false): Code {
let typeName = rawRequestType(ctx, methodDesc);

if (partial) {
typeName = code`${ctx.utils.DeepPartial}<${typeName}>`;
}

if (methodDesc.clientStreaming) {
return code`${imp('Observable@rxjs')}<${typeName}>`;
}
Expand Down

0 comments on commit 0c83892

Please sign in to comment.