Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extending the timeout #26

Open
drmrbrewer opened this issue Mar 18, 2017 · 7 comments
Open

Extending the timeout #26

drmrbrewer opened this issue Mar 18, 2017 · 7 comments
Assignees

Comments

@drmrbrewer
Copy link

So far as I understand it, the timeout value is set when you add the connect-timeout middleware when initializing the express server, so the same timeout applies to every request the comes in thereafter (that matches the route concerned).

What I'm finding is that for some incoming GET requests, there is a particular combination of options in the request that causes a network fetch to a remote resource than is not very reliable. For those requests, I'd like to relax the timeout value for connect-timeout somewhat, to allow for a few fetch retries.

But I can't see any way to change the timeout value 'on the fly', for a particular request. The only control I can see over the timeout is to use req.clearTimeout() whilst processing a request. This is helpful, but timeout is completely removed and will not fire for this request in the future. This is not entirely what I'm looking to achieve... I would like to retain a timeout, but just a slightly longer one.

Is what I'm asking for really naughty, breaking established rules of good practice of which I'm not aware? Or is it a reasonable thing to want to do?

@dougwilson
Copy link
Contributor

Hi @drmrbrewer that is defiantly an interesting use-case. Without understanding more about how your code is set up, it's hard to really say if it's a bad thing to do ;)

The idea is interesting enough I thought maybe to just add it, but since this module uses a setTimeout object under the hood, that object does not provide any way to ether (a) extend the timeout or (b) get the elapsed time it has seen, which certainly would make implement this difficult.

@drmrbrewer
Copy link
Author

Thanks for considering. It would even be useful just to cancel the existing setTimeout() (which is I assume what is presently possible via req.clearTimeout()) and re-start a new setTimeout() using a different timeout value to the one used originally. That way, when a new request comes in that I know will need a little more time, I can (before I've started really processing the request) just set up a new (longer) timeout.

@dougwilson
Copy link
Contributor

haha, I thought of that solution as well, and oops, forgot to mention it above. Yea, that is a good solution; I'll definitely work to implement, but not 100% sure if it'll be today, so please feel free to work on that implementation :) AFAIK, req.setTimeout method already exists, so will probably need to refactor some stuff not to shadow that method.

@rohithkd
Copy link

@dougwilson any update on that? If connect-timeout can override the default behavior, it would be very helpful.

@dougwilson
Copy link
Contributor

I did a tiny bit of work on this a while back, but I think there was some complications and I shelved the work and forgot about it.

@UziTech
Copy link

UziTech commented Jun 13, 2018

This would help me out as well. I created PR #34 that adds 3 methods.

req.setTimeout(delay): reset the timeout and start from now
req.addTimeout(delay): add to the current timeout
req.timeoutLeft(): get the amount of milliseconds left (0 after timedout)

@JamesMGreene
Copy link

See express-timeout-handler alternative implementation with the ability to override the timeout. 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants