fix: add timeout checking docker socket #3279
Conversation
Narrowed issue #3272 down to a missing timeout in isDisguisedPodman(). It looks like the socket is there but never responds, which means that we never allow the Podman extension to start. Signed-off-by: Tim deBoer <git@tdeboer.ca>
deboer-tim
requested review from
jeffmaury and
lstocchi
and removed request for
a team
| // do nothing | ||
| }); | ||
| const req = http | ||
| .get(podmanPingUrl, res => { |
There was a problem hiding this comment.
I would probably use the timeout option of the get method
https://nodejs.org/api/http.html#httprequestoptions-callback
I would also file an issue about limiting the time the activate method of an extension can take
For example activation of an extension should not take more than 5s.
There was a problem hiding this comment.
I would probably use the timeout option of the get method https://nodejs.org/api/http.html#httprequestoptions-callback
I had tried adding timeout: 5000 in podmanPingUrl and listening for timeout events within the get() first, but I couldn't get it to actually timeout; adding the separate setTimeout() worked. When googling all the examples (even after the timeout property was 'recently' added) were doing the same thing.
Seems I'm not the only one - sporadic reports here, mostly for the timeout option: nodejs/node#23282
I would also file an issue about limiting the time the activate method of an extension can take For example activation of an extension should not take more than 5s.
#3280 opened.
|
General issue split off to #3280. |
|
@deboer-tim as requested, I looked and I provided #3497 using the timeout option |
|
Closing in favour of #3497. |
What does this PR do?
Narrowed issue #3272 down to a missing timeout in isDisguisedPodman(). It looks like the socket is there but never responds, which means that we never allow the Podman extension to start. From googling Node HTTP does not appear to have any default timeout (and if it does, I've waited minutes).
Added a 5s timeout. Maybe there is a better values, but this gives it a chance even on a slower machine, and the 5s delay for full startup isn't noticeable.
I presume we should put timeouts on most (all?) of our HTTP requests just in case we ever hit similar problems? I could take a pass via another PR.
Screenshot/screencast of this PR
N/A
What issues does this PR fix or reference?
Fixes #3272.
How to test this PR?
Confirm Podman extension starts up normally.