pnxtech / hydra-router

A service aware router for Hydra Services. Implements an API Gateway and can route web socket messages.
MIT License
60 stars 24 forks source link

Hydra-router request timeout is not passed to Hydra Core #178

Closed edcjrh closed 5 years ago

edcjrh commented 6 years ago

The value for request timeout can be set in the config file, but when the call is created using Hydra core the options.timeout is not present so the default of 30 secs is used. However there are some processes that can take longer than 30 secs. Am I interpreting correctly this parameter? if that is the case then when this call is done:

hydra.makeAPIRequest(msg, {timeout: this.requestTimeout})

The msg will be passed to

makeAPIRequest(message) { return super._makeAPIRequest(message); }

But not the timeout, so the actual value in Hydra core will be NaN therefore the value selected will be: REQUEST_TIMEOUT = 30000;

socket.setTimeout(options.timeout * 1000 || REQUEST_TIMEOUT, () => {

cjus commented 6 years ago

@edcjrh You're absolutely correct. Would you like to issue a pull request or would you like us to update the makeAPIRequest(message) function signature to allow for sendOpts to be passed? This is already present in the _makeAPIRequest(message, sendOpts = { }) signature.

edcjrh commented 6 years ago

Hi Carlos,

It would be ok if you update the function to allow fro sendOpts to be passed.

Thank you!!

Regards,

Juan


From: Carlos Justiniano notifications@github.com Sent: Wednesday, May 23, 2018 4:34 PM To: flywheelsports/hydra-router Cc: Juan Huertas; Mention Subject: Re: [flywheelsports/hydra-router] Hydra-router request timeout is not passed to Hydra Core (#178)

@edcjrhhttps://github.com/edcjrh You're absolutely correct. Would you like to issue a pull request or would you like us to update the makeAPIRequest(message) function signature to allow for sendOpts to be passed? This is already present in the _makeAPIRequest(message, sendOpts = { }) signature.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/flywheelsports/hydra-router/issues/178#issuecomment-391414749, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ASudVqQBkubOh8RWJ2g0j7uk3Kp1Gnabks5t1Y-zgaJpZM4UKVI1.

cjus commented 6 years ago

@edcjrh the fix is now available in hydra:1.6.8 and hydra-express:1.6.9 . I haven't updated Hydra-Router to use hydra.1.6.8 as we have a new hydra-router currently in test. Feel free to update your hydra-router package.json in the meantime. Thanks for pointing out this issue.

edcjrh commented 5 years ago

Sorry for the really late comment. The fix works perfectly, thanks!