Respect request context when wating for picker to initialize#89
Conversation
jhump
left a comment
There was a problem hiding this comment.
This change makes sense! But did you actually encounter a situation where the picker could never initialize and requests hang forever? Is that indicative of another bug regarding the picker never initializing (or potentially a doc improvement or other UX improvement to prevent such issues)?
jhump
left a comment
There was a problem hiding this comment.
I'll go ahead and merge, but I would appreciate add'l input on how you came across this -- both to understand if there are any other sharp edges we can file down in the API or behavior but also to get a more clear idea of how a repro test could be formulated as a follow-up to this change.
Yes I did, and it was caused by a bug in our codebase that make it be used before it was properly initialized. It was considerably harder to find because it hung insead of returning an error. I guess this could potentially be fixed in a better way by returning another error up front when it isn't intialized? But I am not that familiar with the code base, so not sure what to do. This felt like a much less intrusive fix. |
Co-authored-by: Joshua Humphries <2035234+jhump@users.noreply.github.com>
If the transport pool doesn't get properly initialized a http requests will hang forever. This prevents this by respecting the context. It will improve the failure mode so it doesn't accumulate goroutines and eventually prevents other parts from working.