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.

Sign up for GitHub

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

Jump to bottom

Improve server-related types exported by grpc-js #972

Merged
merged 3 commits into from Jul 29, 2019
Merged

Improve server-related types exported by grpc-js #972

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
201dab7
Improve server-related types exported by grpc-js
murgatroid99 Jul 16, 2019
0dc0b68
Fix typo in native types file
murgatroid99 Jul 16, 2019
80d7057
Use typeof instead of util functions
murgatroid99 Jul 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/grpc-js/src/index.ts
View file
Open in desktop
Expand Up @@ -38,9 +38,10 @@ import {
Serialize,
} from './make-client';
import { Metadata } from './metadata';
import { Server } from './server';
import { Server, UntypedHandleCall, UntypedServiceImplementation } from './server';
import { KeyCertPair, ServerCredentials } from './server-credentials';
import { StatusBuilder } from './status-builder';
import { ServerUnaryCall, ServerReadableStream, ServerWritableStream, ServerDuplexStream } from './server-call';

const supportedNodeVersions = require('../../package.json').engines.node;
if (!semver.satisfies(process.version, supportedNodeVersions)) {
Expand Down Expand Up @@ -213,6 +214,12 @@ export {
CallOptions,
StatusObject,
ServiceError,
ServerUnaryCall,
ServerReadableStream,
ServerWritableStream,
ServerDuplexStream,
UntypedHandleCall,
UntypedServiceImplementation
};

/* tslint:disable:no-any */
Expand Down
37 changes: 21 additions & 16 deletions packages/grpc-js/src/server-call.ts
View file
Open in desktop
Expand Up @@ -18,13 +18,14 @@
import { EventEmitter } from 'events';
import * as http2 from 'http2';
import { Duplex, Readable, Writable } from 'stream';
import * as util from 'util';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util.isNumber() and util.isString() are deprecated in favor of typeof value === 'number' and typeof value === 'string', respectively.


import { ServiceError } from './call';
import { StatusObject } from './call-stream';
import { Status } from './constants';
import { Deserialize, Serialize } from './make-client';
import { Metadata } from './metadata';
import { StreamDecoder } from './stream-decoder';
import { ObjectReadable, ObjectWritable } from './object-stream';

interface DeadlineUnitIndexSignature {
[name: string]: number;
Expand Down Expand Up @@ -56,6 +57,10 @@ const defaultResponseOptions = {
waitForTrailers: true,
} as http2.ServerStreamResponseOptions;

export type ServerStatusResponse = Partial<StatusObject>;

export type ServerErrorResponse = ServerStatusResponse & Error;

export type ServerSurfaceCall = {
cancelled: boolean;
getPeer(): string;
Expand All @@ -68,13 +73,13 @@ export type ServerUnaryCall<RequestType, ResponseType> = ServerSurfaceCall & {
export type ServerReadableStream<
RequestType,
ResponseType
> = ServerSurfaceCall & Readable;
> = ServerSurfaceCall & ObjectReadable<RequestType>;
export type ServerWritableStream<
RequestType,
ResponseType
> = ServerSurfaceCall & Writable & { request: RequestType | null };
> = ServerSurfaceCall & ObjectWritable<ResponseType> & { request: RequestType | null };
export type ServerDuplexStream<RequestType, ResponseType> = ServerSurfaceCall &
Duplex;
ObjectReadable<RequestType> & ObjectWritable<ResponseType>;

export class ServerUnaryCallImpl<RequestType, ResponseType> extends EventEmitter
implements ServerUnaryCall<RequestType, ResponseType> {
Expand Down Expand Up @@ -152,7 +157,7 @@ export class ServerWritableStreamImpl<RequestType, ResponseType>
this.call.setupSurfaceCall(this);

this.on('error', err => {
this.call.sendError(err as ServiceError);
this.call.sendError(err);
this.end();
});
}
Expand Down Expand Up @@ -223,7 +228,7 @@ export class ServerDuplexStreamImpl<RequestType, ResponseType> extends Duplex
this.call.setupReadable(this);

this.on('error', err => {
this.call.sendError(err as ServiceError);
this.call.sendError(err);
this.end();
});
}
Expand All @@ -247,7 +252,7 @@ ServerDuplexStreamImpl.prototype.end = ServerWritableStreamImpl.prototype.end;

// Unary response callback signature.
export type sendUnaryData<ResponseType> = (
error: ServiceError | null,
error: ServerErrorResponse | ServerStatusResponse | null,
value: ResponseType | null,
trailer?: Metadata,
flags?: number
Expand Down Expand Up @@ -339,7 +344,7 @@ export class Http2ServerCallStream<
) {
super();

this.stream.once('error', (err: ServiceError) => {
this.stream.once('error', (err: ServerErrorResponse) => {
err.code = Status.INTERNAL;
this.sendError(err);
});
Expand Down Expand Up @@ -379,7 +384,7 @@ export class Http2ServerCallStream<
const match = timeoutHeader[0].toString().match(DEADLINE_REGEX);

if (match === null) {
const err = new Error('Invalid deadline') as ServiceError;
const err = new Error('Invalid deadline') as ServerErrorResponse;
err.code = Status.OUT_OF_RANGE;
this.sendError(err);
return;
Expand Down Expand Up @@ -439,7 +444,7 @@ export class Http2ServerCallStream<
}

async sendUnaryMessage(
err: ServiceError | null,
err: ServerErrorResponse | ServerStatusResponse | null,
value: ResponseType | null,
metadata?: Metadata,
flags?: number
Expand Down Expand Up @@ -490,21 +495,21 @@ export class Http2ServerCallStream<
}
}

sendError(error: ServiceError) {
sendError(error: ServerErrorResponse | ServerStatusResponse) {
const status: StatusObject = {
code: Status.UNKNOWN,
details: error.hasOwnProperty('message')
details: ('message' in error)
? error.message
: 'Unknown Error',
metadata: error.hasOwnProperty('metadata')
metadata: ('metadata' in error && error.metadata !== undefined)
? error.metadata
: new Metadata(),
};

if (error.hasOwnProperty('code') && Number.isInteger(error.code)) {
if ('code' in error && util.isNumber(error.code) && Number.isInteger(error.code)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The util.isNumber() (or typeof error.code === 'number') check shouldn't be necessary due to the Number.isInteger() call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Typescript was not happy with calling Number.isInteger on that value without verifying that it was a number.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right: microsoft/TypeScript#21199 😕

status.code = error.code;

if (error.hasOwnProperty('details')) {
if ('details' in error && util.isString(error.details)) {
status.details = error.details;
}
}
Expand Down Expand Up @@ -637,7 +642,7 @@ export class Http2ServerCallStream<
type UntypedServerCall = Http2ServerCallStream<any, any>;

function handleExpiredDeadline(call: UntypedServerCall) {
const err = new Error('Deadline exceeded') as ServiceError;
const err = new Error('Deadline exceeded') as ServerErrorResponse;
err.code = Status.DEADLINE_EXCEEDED;

call.sendError(err);
Expand Down
18 changes: 9 additions & 9 deletions packages/grpc-js/src/server.ts
View file
Open in desktop
Expand Up @@ -41,6 +41,8 @@ import {
ServerWritableStream,
ServerWritableStreamImpl,
UnaryHandler,
ServerErrorResponse,
ServerStatusResponse,
} from './server-call';
import { ServerCredentials } from './server-credentials';

Expand All @@ -57,9 +59,9 @@ type UntypedUnaryHandler = UnaryHandler<any, any>;
type UntypedClientStreamingHandler = ClientStreamingHandler<any, any>;
type UntypedServerStreamingHandler = ServerStreamingHandler<any, any>;
type UntypedBidiStreamingHandler = BidiStreamingHandler<any, any>;
type UntypedHandleCall = HandleCall<any, any>;
export type UntypedHandleCall = HandleCall<any, any>;
type UntypedHandler = Handler<any, any>;
interface UntypedServiceImplementation {
export interface UntypedServiceImplementation {
[name: string]: UntypedHandleCall;
}

Expand Down Expand Up @@ -100,7 +102,7 @@ export class Server {
throw new Error('Not implemented. Use addService() instead');
}

addService(service: ServiceDefinition, implementation: object): void {
addService(service: ServiceDefinition, implementation: UntypedServiceImplementation): void {
if (this.started === true) {
throw new Error("Can't add a service to a started server.");
}
Expand All @@ -120,8 +122,6 @@ export class Server {
throw new Error('Cannot add an empty service to a server');
}

const implMap: UntypedServiceImplementation = implementation as UntypedServiceImplementation;

serviceKeys.forEach(name => {
const attrs = service[name];
let methodType: HandlerType;
Expand All @@ -140,11 +140,11 @@ export class Server {
}
}

let implFn = implMap[name];
let implFn = implementation[name];
let impl;

if (implFn === undefined && typeof attrs.originalName === 'string') {
implFn = implMap[attrs.originalName];
implFn = implementation[attrs.originalName];
}

if (implFn !== undefined) {
Expand Down Expand Up @@ -414,7 +414,7 @@ async function handleUnary<RequestType, ResponseType>(
handler.func(
emitter,
(
err: ServiceError | null,
err: ServerErrorResponse | ServerStatusResponse | null,
value: ResponseType | null,
trailer?: Metadata,
flags?: number
Expand All @@ -436,7 +436,7 @@ function handleClientStreaming<RequestType, ResponseType>(
);

function respond(
err: ServiceError | null,
err: ServerErrorResponse | ServerStatusResponse | null,
value: ResponseType | null,
trailer?: Metadata,
flags?: number
Expand Down
8 changes: 6 additions & 2 deletions packages/grpc-native-core/index.d.ts
View file
Open in desktop
Expand Up @@ -412,13 +412,13 @@ declare module "grpc" {
* User provided method to handle server streaming methods on the server.
*/
type handleServerStreamingCall<RequestType, ResponseType> =
(call: ServerWriteableStream<RequestType>) => void;
(call: ServerWritableStream<RequestType>) => void;

/**
* A stream that the server can write to. Used for calls that are streaming
* from the server side.
*/
export class ServerWriteableStream<RequestType> extends Writable {
export class ServerWritableStream<RequestType> extends Writable {
/**
* Indicates if the call has been cancelled
*/
Expand Down Expand Up @@ -449,6 +449,10 @@ declare module "grpc" {
sendMetadata(responseMetadata: Metadata): void;
}

/* This typo existed in previous versions of this file, so we provide this
* type alias for backwards compatibility. */
export type ServerWriteableStream<RequestType> = ServerWritableStream<RequestType>;

/**
* User provided method to handle bidirectional streaming calls on the server.
*/
Expand Down