[Colossus] fix: upload aborted error#5094
Conversation
mnaamani
left a comment
There was a problem hiding this comment.
Great that you found this settings. I suppose we should have looked more closely at changes between v14 and v18 that would have impacted our applications. Let keep that in mind when we shift to v20+
If we want a quick deployment of this fix because we are seeing many failed uploads this is good to go. Just need to bump colossus version.
|
|
||
| // INFO: https://nodejs.org/dist/latest-v18.x/docs/api/http.html#serverrequesttimeout | ||
| // Set the server request timeout to 0 to disable it. This was default behaviour pre Node.js 18.x | ||
| server.requestTimeout = 0 |
There was a problem hiding this comment.
I think it makes sense to have the timeout disabled, provided the node is always behind a reverse proxy and not exposed on a public interface.
Alternatively we can have a non zero timeout, but larger value of say 20min, better than default of 5min?
There was a problem hiding this comment.
Or disabled timeout but make sure docs and our default deployment configs do not expose public interface.. ?
There was a problem hiding this comment.
My initial idea was to set a env var as a parameter named MAX_REQUEST_TIMEOUT = 20 * 60 * 1000 (20 mins in milliseconds) so that it can be set by the user.
|
The fix works in the sense that the above observed error is gone and the request time is in my case 6.4 mins > 5mins |
dobertRowneySr
left a comment
There was a problem hiding this comment.
LGTM, I have also tested the fix locally
addresses #5081
@types/nodedependency version to18.6.0Context
There is a server.requestTimeout configuration option in the
http.Serverinstance (rememberexpressis just a wrapper around nodejs native HTTP Server), from the nodejs official documentation what this option means:The default value for the option is 300s for node versions
>=18.x, while for the previous version the default value was 0 (no timeout)So, the uploads failing problem started to occur when we updated the Nodejs from
v14tov18.6.0in #4778This timeout value of 300s also validates Ignazio's investigation that Colossus aborts errors after ~5 minutes.