grpc-caller icon indicating copy to clipboard operation
grpc-caller copied to clipboard

Swap options

Open smousa opened this issue 5 years ago • 2 comments

Fixed some small issues related to passing in a grpc proto as opposed to reading it in from a file.

smousa avatar Jul 11 '19 16:07 smousa

Hello, thank you for the PR. Can you elaborate the issue this addresses? I agree with the change of order of assignments, but why are we removing the options param from the underlying client instructor invocation? Also I don't think the tests need any adjustments?

bojand avatar Jul 12 '19 13:07 bojand

It looked like the retry code doesn't actually get used until it gets into client.js

I was running into an issue where my proto client was calling new grpc.Channel() and didn't know what to do with the retry arguments, because it was expecting something completely different.

I had changed the test, because the "defaults" field (which is the last argument) should contain both the metadata and the options, like,

{
    metadata: { foo: 'bar'},
    options: {
        interceptors: [ myverycoolinterceptor ]
    }
}

But in this test you were storing the interceptor config in the retry options field, which was failing, if you look at the previous commit. I am not sure why your tests didn't fail before, nor did I investigate that too deeply, but yeah.

I have independently verified this code works against the stuff I am working on, but I am using the alternate implementation where I am passing in the proto client, so maybe my proto client is different????

I hope this helps. Please let me know if you have any other questions.

smousa avatar Jul 12 '19 23:07 smousa