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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting reponse timeout handler throws an error if request is sent in through FastifyInstance.inject #74

Closed
gyszalai opened this issue Dec 11, 2019 · 5 comments

Comments

@gyszalai
Copy link
Contributor

馃悰 Bug Report

It seems that fastify-http-proxy does not allow to set a response timeout handler if the original request is sent in to fastify throug FastifyInstance.inject

To Reproduce

Assume I have the following configuration:

const proxy = require('fastify-http-proxy')
const fastify = ...
fastify.register(proxy, {
  upstream: upstreamUrl,
  prefix: '/myPrefix',
  http2: false,
  async preHandler(request, reply) {
    reply.res.setTimeout(5000, () => {
      reply.status(502).send('Gateway timeout')
    })
  }
})

I have a test case that would test if the timeout is working:

it('should respond with HTTP 502 if the proxied system does not respond in the specified timeout', async () => {
  // mock server
  nock('http://localhost:9000')
      .get('/')
      .delay(6000)
      .reply(200, { dummy: 'dummy' })
  // call
  const response = await fastify
      .inject({ method: 'GET', url: '/myPrefix' })
      .should.be.fulfilled()
  // test
  should(response.statusCode).be.equal(502)
})

Expected behavior

The test case should be successful, but it fails with the following error:

This test fails with the followig error:

TypeError: this.socket.setTimeout is not a function
    at Response.setTimeout (_http_outgoing.js:222:17)
    at Object.preHandler (<project root>/src/my-route.ts:34:27)
    at hookIterator (<project root>/node_modules/fastify/lib/hooks.js:124:10)
    at next (<project root>/node_modules/fastify/lib/hooks.js:70:20)
    at hookRunner (<project root>/node_modules/fastify/lib/hooks.js:84:3)
    at preValidationCallback (<project root>/node_modules/fastify/lib/handleRequest.js:92:5)
    at handler (<project root>/node_modules/fastify/lib/handleRequest.js:69:5)
    at handleRequest (<project root>/node_modules/fastify/lib/handleRequest.js:18:5)
    at onRunMiddlewares (<project root>/node_modules/fastify/lib/middleware.js:22:5)
    at Holder.done (<project root>/node_modules/middie/middie.js:90:9)
    at xXssProtection (<project root>/node_modules/x-xss-protection/dist/index.js:47:13)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at crossdomain (<project root>/node_modules/helmet-crossdomain/dist/index.js:27:9)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at nosniff (<project root>/node_modules/dont-sniff-mimetype/dist/index.js:5:9)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at ienoopen (<project root>/node_modules/ienoopen/dist/index.js:5:9)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at hsts (<project root>/node_modules/hsts/index.js:52:5)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at hidePoweredBy (<project root>/node_modules/hide-powered-by/dist/index.js:7:13)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at frameguard (<project root>/node_modules/frameguard/dist/index.js:55:9)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at dnsPrefetchControl (<project root>/node_modules/dns-prefetch-control/dist/index.js:14:9)
    at Holder.done (<project root>/node_modules/middie/middie.js:112:11)
    at Object.run (<project root>/node_modules/middie/middie.js:59:12)
    at middlewareCallback (<project root>/node_modules/fastify/lib/route.js:384:27)
    at next (<project root>/node_modules/fastify/lib/hooks.js:66:7)
    at handleResolve (<project root>/node_modules/fastify/lib/hooks.js:77:5)

Your Environment

  • node version: 12.13.1
  • fastify version: >=2.11.0
  • os: Mac
@gyszalai gyszalai changed the title Setting reponse timeout handler throws an error if request is sent in though FastifyInstance.inject Setting reponse timeout handler throws an error if request is sent in through FastifyInstance.inject Dec 11, 2019
@mcollina
Copy link
Member

Good find! I think this is a bug in https://github.com/fastify/light-my-request (which powers inject()). We should override the setTimeout function in https://nodejs.org/dist/latest-v12.x/docs/api/http.html#http_response_settimeout_msecs_callback and provide adeguate behavior. Would you like to send a PR for that?

Note that this has nothing to do with fastify-http-proxy

@gyszalai
Copy link
Contributor Author

Just finished it, i'll file a PR in minutes

@gyszalai
Copy link
Contributor Author

@gyszalai
Copy link
Contributor Author

@mcollina shall I close this issue or wait until the PR gets merged?

@mcollina
Copy link
Member

Yes, we can close this.

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

No branches or pull requests

2 participants