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

fix: fix destructing error if there is no call.method.options #291

Closed

Conversation

nikolaymatrosov
Copy link

If protobuf files were compiled without --ts_proto_opt=outputServices=generic-definitions, there will be no call.method.options. As a result, trying to destructure it and get the idempotencyLevel value will result in the following error.

TypeError: Cannot destructure property 'idempotencyLevel' of 'call.method.options' as it is undefined.

The fix makes the retry middleware usable for more people who use different ts_proto options and are willing to manually provide retry options to API calls.

@aikoven
Copy link
Contributor

aikoven commented Feb 15, 2023

Thanks for your PR!

The call.method.options is a required property, see the type of MethodDescriptor:

options: {
idempotencyLevel?: 'IDEMPOTENT' | 'NO_SIDE_EFFECTS';
};

Which means that this object is created incorrectly on the other side. Are you using ts-proto with grpc-js service definitions? Could you share your full ts_proto_opt?

@nikolaymatrosov
Copy link
Author

Yes, in the project that I am having issues with ts_proto is called with grpc-js option. https://github.com/yandex-cloud/nodejs-sdk/blob/master/scripts/generate-code.ts#L27

@nikolaymatrosov
Copy link
Author

So you're saying I need to fix options and make the object optional to match behavior of grpc-js?

@aikoven
Copy link
Contributor

aikoven commented Feb 15, 2023

No, we should rather find out the place where method descriptor is generated from grpc-js descriptor and add the missing field to match the type.

@aikoven aikoven closed this in #305 Mar 5, 2023
aikoven added a commit that referenced this pull request Mar 5, 2023
…305)

Prior to this fix the library incorrectly detected `grpc-js` service
definitions generated by `ts-proto` as `ts-proto` generic definitions.

Closes #291
@aikoven
Copy link
Contributor

aikoven commented Mar 5, 2023

Apparently the ts-proto-generated grpc-js service definitions were incorrectly detected as ts-proto definitions. Fixed in #305, gonna release soon.

Thank you for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants