Timeout errors with clever deploy (the git push part)

[hsablonniere]

Thanks to @JulienBrgs, we identified a very nasty bug:

🤯 The problem

Sometimes clever deploy fails with a timeout during the git push phase.

• It happens with Node.js 20 but not with Node.js 18.
• We thought it only happened with big payloads (when lots of commits and data needs to be pushed) but it's not really clear.

🧐 The details

Here's what happens:

• The clever deploy command uses https://github.com/isomorphic-git/isomorphic-git to push commits.
• isomorphic-git does the git push via an HTTP POST using https://github.com/feross/simple-get.
• simple-get uses Node's low level http.request function to do this HTTP POST.

Why does it work with Node.js 18 but not with Node.js 20?

• When you call http.request, Node.js uses the http.globalAgent.
• In Node.js 18, the options of this default global agent are keepAlive: false and no timeout.
• In Node.js 19, they changed the options to keepAlive: true and timeout: 5000.
  • Set a default (http|https).Agent with keep-alive nodejs/node#37184

Our "timeout bug" is caused by this Node.js update but why would isomorphic-git (via simple-get) get a timeout error if it's genuinely pushing data with an HTTP POST?

I wrote a reduced test case with a basic HTTP server and some POST calls using simple-get to confirm the origin:

• With Node.js 18, you can trigger the timeout error by using an agent with the same settings as Node.js 20 (keepAlive: true and timeout: 5000).
• With Node.js 20, you can prevent the timeout error by using an agent without the timeout option.
  • The keepAlive: true is not the cause of the timeout.
  • You could also use agent: false with http.request, but it would prevent the different requests to reuse the same HTTP socket/connection.

Then I tried with an agent configured with a timeout of 5 seconds and different server behaviours to understand what situations trigger the timeout error.

• If the server hasn't sent the response status yet, but refuses to receive any data from the request body for more than 5 secondes, the client will get a timeout error.
• If the server has already sent the response status, but refuses to send a response body and finish the response, it will trigger a timeout on the client.

After discussing this with my colleagues, we think our git server sometimes wait a bit too much before accepting the request body (to validate the auth or something).
It's rare but it can happen in some situations so we need to remove this 5 seconds timeout option introduced in Node.js 19.

💡 The solutions

• (1) Ask simple-get to use a default agent without a timeout
  • We'll have to do a PR, wait for a merge, a release and then the same for isomorphic-git as it's a transitive dep
  • This may not be the behaviour other users want
• (2) Ask isomorphic-git to pass an agent without a timeout to simple-get
  • We'll have to do a PR, wait for a merge and a release
  • This may not be the behaviour other users want
• (3) Modify the http.globalAgent temporarily before/after the git push
  • This seems like the simplest solution but it looks a bit like a hack
• (4) Create an http implementation for isomorphic-git that does not have this timeout setting
  • This doesn't feel that hard to do https://isomorphic-git.org/docs/en/http#implementing-your-own-http-client
  • Seems like @mojavelinux exposed the agent because he needed a proxy, that could help https://github.com/isomorphic-git/isomorphic-git/releases/tag/v1.12.0

➡️ Let's go for solution (4).