core (grpc) use retry strategy#1273
Conversation
googlebot
added
the
cla: yes
stephenplusplus
force-pushed
the
spp--1265
branch
2 times, most recently
from
cc1a30d to
dea768a
Compare
stephenplusplus
force-pushed
the
spp--1265
branch
from
dea768a to
9bfe432
Compare
|
@callmehiphop this is just another option, didn't mean for it to auto-invalidate your work. If you don't think this approach is the right way to go, just let me know! I'll wait before I get the tests ready. |
|
It's cool, this was the first response I'd seen from you in regards to this issue in several days, so I figured you weren't a fan of what I was propsing. I originally did start going down this route, but I felt like the nesting/wrapping increased the cyclomatic complexity, which is why I decided to abandon it. If you think this is the best way to go, that's fine with me. |
|
It actually wasn't that I wasn't a fan, it was just hard to review from the 🚗 😢 👶 situation. Like you, I'm okay with all, but not a super-fan of any. I like this overall because it didn't force changes to retry-request (which I thought would be good originally, but in practice, turns out awkward [as you predicted]). I also like this because it doesn't duplicate any retry logic which already exists in retry-request. I'll get going on some tests. |
When `resp` is undefined, this error handling code will itself throw "TypeError: Cannot read properties of undefined (reading 'insertErrors')" Instead of throwing, we can use optional chaining to allow the error message to be handled as expected. Co-authored-by: Lo Ferris <50979514+loferris@users.noreply.github.com>
4 participants
Fixes #1265
To Dos
// @callmehiphop