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

Fix grpc channel options #8365

Merged
merged 5 commits into from Nov 8, 2021
Merged

Fix grpc channel options #8365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a96ceb4
Merge pull request #1 from nestjs/master
sjkummer Oct 18, 2021
5ac3637
The GrpcOptions attribute "channelOptions" was not used anymore (neve…
sjkummer Oct 18, 2021
e88e310
cleanup
sjkummer Oct 18, 2021
25dbb33
Alligned gRPC ChannelOptions code in server and client.
sjkummer Oct 19, 2021
fdf11a5
fix(microservices): npm run format
sjkummer Oct 27, 2021
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
46 changes: 17 additions & 29 deletions packages/microservices/client/client-grpc.ts
View file
Open in desktop
Expand Up @@ -2,18 +2,14 @@ import { Logger } from '@nestjs/common/services/logger.service';
import { loadPackage } from '@nestjs/common/utils/load-package.util';
import { isFunction, isObject } from '@nestjs/common/utils/shared.utils';
import { Observable, Subscription } from 'rxjs';
import {
GRPC_DEFAULT_MAX_RECEIVE_MESSAGE_LENGTH,
GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH,
GRPC_DEFAULT_PROTO_LOADER,
GRPC_DEFAULT_URL,
} from '../constants';
import { GRPC_DEFAULT_PROTO_LOADER, GRPC_DEFAULT_URL } from '../constants';
import { InvalidGrpcPackageException } from '../errors/invalid-grpc-package.exception';
import { InvalidGrpcServiceException } from '../errors/invalid-grpc-service.exception';
import { InvalidProtoDefinitionException } from '../errors/invalid-proto-definition.exception';
import { ClientGrpc, GrpcOptions } from '../interfaces';
import { ClientProxy } from './client-proxy';
import { GRPC_CANCELLED } from './constants';
import { ChannelOptions } from '../external/grpc-options.interface';

let grpcPackage: any = {};
let grpcProtoLoaderPackage: any = {};
Expand Down Expand Up @@ -65,33 +61,25 @@ export class ClientGrpcProxy extends ClientProxy implements ClientGrpc {
throw new InvalidGrpcServiceException();
}

const maxSendMessageLengthKey = 'grpc.max_send_message_length';
const maxReceiveMessageLengthKey = 'grpc.max_receive_message_length';
const maxMessageLengthOptions = {
[maxSendMessageLengthKey]: this.getOptionsProp(
this.options,
'maxSendMessageLength',
GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH,
),
[maxReceiveMessageLengthKey]: this.getOptionsProp(
this.options,
'maxReceiveMessageLength',
GRPC_DEFAULT_MAX_RECEIVE_MESSAGE_LENGTH,
),
};
const maxMetadataSize = this.getOptionsProp(
this.options,
'maxMetadataSize',
-1,
);
if (maxMetadataSize > 0) {
maxMessageLengthOptions['grpc.max_metadata_size'] = maxMetadataSize;
const channelOptions: ChannelOptions =
this.options && this.options.channelOptions
? this.options.channelOptions
: {};
if (this.options && this.options.maxSendMessageLength) {
channelOptions['grpc.max_send_message_length'] =
this.options.maxSendMessageLength;
}
if (this.options && this.options.maxReceiveMessageLength) {
channelOptions['grpc.max_receive_message_length'] =
this.options.maxReceiveMessageLength;
}
if (this.options && this.options.maxMetadataSize) {
channelOptions['grpc.max_metadata_size'] = this.options.maxMetadataSize;
}

const keepaliveOptions = this.getKeepaliveOptions();
const options: Record<string, string | number> = {
...(this.options.channelOptions || {}),
...maxMessageLengthOptions,
...channelOptions,
...keepaliveOptions,
};

Expand Down
2 changes: 0 additions & 2 deletions packages/microservices/constants.ts
View file
Open in desktop
Expand Up @@ -39,8 +39,6 @@ export const GRPC_DEFAULT_PROTO_LOADER = '@grpc/proto-loader';
export const NO_MESSAGE_HANDLER = `There is no matching message handler defined in the remote service.`;
export const NO_EVENT_HANDLER = `There is no matching event handler defined in the remote service.`;
export const DISCONNECTED_RMQ_MESSAGE = `Disconnected from RMQ. Trying to reconnect.`;
export const GRPC_DEFAULT_MAX_RECEIVE_MESSAGE_LENGTH = 4 * 1024 * 1024;
export const GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH = 4 * 1024 * 1024;

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 it is worth returning these default values and using them when the developer didn't provide them explicitly in the options.

Copy link
Contributor Author

@sjkummer sjkummer Oct 27, 2021 •
edited

Choose a reason for hiding this comment

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

4 MB is the current gRPC default, therefore nothing changes for developers who did not set a value. Why redeclare? And if the gRPC default changes (for reasons) in the futurore, why would nestJS want to stick to outdated default values?

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha then yep it makes sense to remove these options.

export const KAFKA_DEFAULT_CLIENT = 'nestjs-consumer';
export const KAFKA_DEFAULT_GROUP = 'nestjs-group';
Expand Down
4 changes: 4 additions & 0 deletions packages/microservices/external/grpc-options.interface.ts
View file
Open in desktop
Expand Up @@ -3,6 +3,9 @@
* This listing is incomplete. Full reference: https://grpc.github.io/grpc/core/group__grpc__arg__keys.html
*/
export interface ChannelOptions {
'grpc.max_send_message_length'?: number;
'grpc.max_receive_message_length'?: number;
'grpc.max_metadata_size'?: number;
'grpc.ssl_target_name_override'?: string;
'grpc.primary_user_agent'?: string;
'grpc.secondary_user_agent'?: string;
Expand All @@ -12,5 +15,6 @@ export interface ChannelOptions {
'grpc.initial_reconnect_backoff_ms'?: number;
'grpc.max_reconnect_backoff_ms'?: number;
'grpc.use_local_subchannel_pool'?: number;
'grpc-node.max_session_memory'?: number;
[key: string]: string | number | undefined;
}
4 changes: 3 additions & 1 deletion packages/microservices/interfaces/microservice-configuration.interface.ts
View file
Open in desktop
Expand Up @@ -33,7 +33,9 @@ export interface CustomStrategy {
export interface GrpcOptions {
transport?: Transport.GRPC;
options: {
interceptors?: Array<(options: any, nextCall: (options: any) => any) => any>;
interceptors?: Array<
(options: any, nextCall: (options: any) => any) => any
>;
url?: string;
maxSendMessageLength?: number;
maxReceiveMessageLength?: number;
Expand Down
38 changes: 16 additions & 22 deletions packages/microservices/server/server-grpc.ts
View file
Open in desktop
Expand Up @@ -7,8 +7,6 @@ import { EMPTY, fromEvent, lastValueFrom, Subject } from 'rxjs';
import { catchError, takeUntil } from 'rxjs/operators';
import {
CANCEL_EVENT,
GRPC_DEFAULT_MAX_RECEIVE_MESSAGE_LENGTH,
GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH,
GRPC_DEFAULT_PROTO_LOADER,
GRPC_DEFAULT_URL,
} from '../constants';
Expand All @@ -19,6 +17,7 @@ import { InvalidProtoDefinitionException } from '../errors/invalid-proto-definit
import { CustomTransportStrategy, MessageHandler } from '../interfaces';
import { GrpcOptions } from '../interfaces/microservice-configuration.interface';
import { Server } from './server';
import { ChannelOptions } from '../external/grpc-options.interface';

let grpcPackage: any = {};
let grpcProtoLoaderPackage: any = {};
Expand Down Expand Up @@ -333,27 +332,22 @@ export class ServerGrpc extends Server implements CustomTransportStrategy {
}

public async createClient(): Promise<any> {
const grpcOptions = {
'grpc.max_send_message_length': this.getOptionsProp(
this.options,
'maxSendMessageLength',
GRPC_DEFAULT_MAX_SEND_MESSAGE_LENGTH,
),
'grpc.max_receive_message_length': this.getOptionsProp(
this.options,
'maxReceiveMessageLength',
GRPC_DEFAULT_MAX_RECEIVE_MESSAGE_LENGTH,
),
};
const maxMetadataSize = this.getOptionsProp(
this.options,
'maxMetadataSize',
-1,
);
if (maxMetadataSize > 0) {
grpcOptions['grpc.max_metadata_size'] = maxMetadataSize;
const channelOptions: ChannelOptions =
this.options && this.options.channelOptions
? this.options.channelOptions
: {};
if (this.options && this.options.maxSendMessageLength) {
channelOptions['grpc.max_send_message_length'] =
this.options.maxSendMessageLength;
}
if (this.options && this.options.maxReceiveMessageLength) {
channelOptions['grpc.max_receive_message_length'] =
this.options.maxReceiveMessageLength;
}
if (this.options && this.options.maxMetadataSize) {
channelOptions['grpc.max_metadata_size'] = this.options.maxMetadataSize;
}
const server = new grpcPackage.Server(grpcOptions);
const server = new grpcPackage.Server(channelOptions);
const credentials = this.getOptionsProp(this.options, 'credentials');

await new Promise((resolve, reject) => {
Expand Down