Cannot completely disable requestTimeout and sessionTimeout

[gunters63]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.26.2

Plugin version

9.7.0

Node.js version

v20.11.1

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Ubuntu 20.04

Description

I have an industrial application here which uses grpc-web in the browser to communicate with a .Net backend.
I use a fastify web server as a proxy to forward the HTTP/2 requests (content-type "application/grpc-web-text") to the .Net backend.

After some fiddling I got it to work with the help of @fastify/http-proxy (which uses @fastify/reply-from internally).

But I still have a somewhat exotic problem. The UI makes heavy use of Grpc subscriptions. A Grpc subscription keeps a HTTP/2 session open as long as the subscription is active.

The UI can sit idle for hours and days with the same active subscription(s), which means the default requestTimeout and sessionTimeout timeouts have to be disabled or increased.

Disabling is not possible because the code handles a timeout of 0 as undefined:

  if (!http2Opts.sessionTimeout) {
    http2Opts.sessionTimeout = opts.sessionTimeout || 60000
  }
  if (!http2Opts.requestTimeout) {
    http2Opts.requestTimeout = 10000
  }

I increased the timeouts to the maximal possible values instead (2147483647). Which means after 25 days the UI will fail if no one touches it. Which is unlikely but I don't want to rule it out.

In this use case it would best if you could completely disable the timeouts. So I propose to handle the 0 value explicitely in the code. I would also be happy to provide a merge request if this change is approved.

Link to code that reproduces the bug

No response

Expected Behavior

The user should be able to completely disable requestTimeout and sessionTimeout by providing a value of 0.

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[gunters63]

Okidoki, I prepare one in the next days :)

[gunters63]

@mcollina: I have prepared a draft pull request (#368)

I wrote a test for the disabled request timeout and wanted it to fail with the original code and pass with my changes.
This troubled me a little, because the original code falls back to a default of 10000 ms if requestTimeout is falsy.
So currently the test has a long runtime of 11000ms. I am sure there is a better way, maybe you can give me hint.

I also wanted to write a test for sessionTimeout disabled (this test is not included). But when I disable the session timeout, tap never finished, so I left it out for now.

[gunters63]

One more detail:

The timeouts are used for the client and request object from Nodes http2 API.
It is not explicitly stated in the documentation, but a passed timeout of 0 seems to disable the timeout, so I didn't guard the calls by checking a zero timeout value.