Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

When using onResponse the remote http statusCode is not being passed to fn and therefore all status are 200 #30

Closed
yohay-ma opened this issue Jul 3, 2020 · 2 comments · Fixed by #32

Comments

@yohay-ma
Copy link
Contributor

yohay-ma commented Jul 3, 2020

🐛 Bug Report

When using onResponse the remote http statusCode is not being used and therefore all status are 200

To Reproduce

in opts.test.js extend the service and add an additional route that returns different status code then 200.
add a test that calls this route and expect to get the different status

line 113 in index.js

onResponse(req, res, stream)

Expected behavior

the onResponse should get the response object

onResponse(req, res, stream, response)

Your Environment

  • node version: 12
  • fastify version: latest
  • os: Mac
@yohay-ma
Copy link
Contributor Author

yohay-ma commented Jul 3, 2020

I cannot submit a PR so here you go:
opts.test.js

/* global describe, it */
'use strict'

const request = require('supertest')
const bodyParser = require('body-parser')
const expect = require('chai').expect
const pump = require('pump')
let gateway, service, close, proxy, gHttpServer

const nock = require('nock')

nock('http://dev.com')
  .get('/service/headers?age=33')
  .reply(200, {
  }, {
    url: 'http://dev.com'
  })

describe('fast-proxy smoke', () => {
  it('init', async () => {
    const fastProxy = require('../index')({
      base: 'http://127.0.0.1:3000'
    })
    close = fastProxy.close
    proxy = fastProxy.proxy

    expect(proxy instanceof Function).to.equal(true)
    expect(close instanceof Function).to.equal(true)
  })

  it('init & start gateway', async () => {
    // init gateway
    gateway = require('restana')()

    gateway.all('/service/*', function (req, res) {
      proxy(req, res, req.url, {
        base: req.headers.base,
        queryString: { age: 33 },
        onResponse (req, res, stream, response) {
          res.statusCode = response.statusCode
          pump(stream, res)
        }
      })
    })

    gHttpServer = await gateway.start(8080)
  })

  it('init & start remote service', async () => {
    // init remote service
    service = require('restana')()
    service.use(bodyParser.json())

    service
      .get('/service/headers', (req, res) => {
        res.setHeader('url', req.url)
        res.send()
      })
      .get('/service/302', (req, res) => {
        res.statusCode = 302
        res.end()
      })

    await service.start(3000)
  })

  it('should 200 on GET /servive/headers + opts', async () => {
    await request(gHttpServer)
      .get('/service/headers')
      .expect(200)
      .then((response) => {
        expect(response.headers.url).to.equal('/service/headers?age=33')
      })
  })

  it('should overwrite global base', async () => {
    await request(gHttpServer)
      .get('/service/headers')
      .set('base', 'http://localhost:3000')
      .expect(200)
      .then((response) => {
        expect(response.headers.url).to.equal('/service/headers?age=33')
      })
  })

  it('should overwrite global base 2', async () => {
    await request(gHttpServer)
      .get('/service/headers')
      .set('base', 'http://dev.com')
      .then((response) => {
        expect(response.headers.url).to.equal('http://dev.com')
      })
  })

  it('should support http status in on response', async () => {
    await request(gHttpServer)
      .get('/service/302')
      .expect(302)
  })

  it('close all', async () => {
    close()
    await gateway.close()
    await service.close()
  })
})

index.js line 113

onResponse(req, res, stream, response)

also add to .gitignore

# jetbrains webstorm
.idea

@jkyberneees
Copy link
Collaborator

jkyberneees commented Jul 3, 2020

Hi @yohay-ma, thanks for reporting this issue.

You are right, at the moment it is no clear how to reference the origin response code (it is actually part of the stream object).

See example: https://github.com/jkyberneees/fast-gateway/blob/master/lib/default-hooks.js#L41

I have submitted a PR in order to preset the origin response code by default in #32, this simplifies custom onResponse hook implementations as you just pointed out.

Best Regards.

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

Successfully merging a pull request may close this issue.

2 participants