-
Notifications
You must be signed in to change notification settings - Fork 679
grpc-js-core: more changes #128
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,7 +79,14 @@ class SecureChannelCredentialsImpl extends ChannelCredentialsImpl { | |
} | ||
} | ||
|
||
function verifyIsBufferOrNull(obj: any, friendlyName: string): void { | ||
if (obj && !(obj instanceof Buffer)) { | ||
throw new TypeError(`${friendlyName}, if provided, must be a Buffer.`); | ||
} | ||
} | ||
|
||
export namespace ChannelCredentials { | ||
|
||
/** | ||
* Return a new ChannelCredentials instance with a given set of credentials. | ||
* The resulting instance can be used to construct a Channel that communicates | ||
|
@@ -91,6 +98,9 @@ export namespace ChannelCredentials { | |
export function createSsl( | ||
rootCerts?: Buffer|null, privateKey?: Buffer|null, | ||
certChain?: Buffer|null): ChannelCredentials { | ||
verifyIsBufferOrNull(rootCerts, 'Root certificate'); | ||
verifyIsBufferOrNull(privateKey, 'Private key'); | ||
verifyIsBufferOrNull(certChain, 'Certificate chain'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these not redundant with the argument type annotations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TS doesn't auto-generate sentinel checks in JS, its typings are only enforced for other TS files. The credentials test depends on these checks existing, and is written in JS. |
||
if (privateKey && !certChain) { | ||
throw new Error( | ||
'Private key must be given with accompanying certificate chain'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes to this file do not match the existing implementation, which only emits the status event for calls with response streaming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like that's not the case -- this test depends on
'status'
being emitted for a unary call:grpc-node/test/api/surface_test.js
Lines 896 to 904 in ac186ec