expressjs / compression

Node.js compression middleware
MIT License
2.77k stars 241 forks source link

when using supertest, there is no signs of compression working in res.headers #102

Closed faceyspacey closed 7 years ago

faceyspacey commented 7 years ago

For example, I have a simple setup like this:

server.js:

app.use(compression())

app.get('*', function(req, res) {
  handleRender(req, res)
     .catch(error => console.log('SSR ERROR', error))
})

const server = http.createServer(app)

server.listen(process.env.PORT || 3000, function() {
  console.log('Listening on %j', server.address())
})

test.js:

it('REQUEST: /some-path' (done) => {
    const server = startServer()

    request(server)
       .get('/some-path)
       .set('Accept-Encoding', 'gzip') // i've tried with and without this line (i'm not sure why I should have to add it since that's what the compression should be doing)
       .expect('Content-Encoding', 'gzip') // this line causes it it to fail
       .end((err, res) => {
          if(err) done.fail(err)
          else done()
       })
})

I'm looking for some proof in my tests that the compression is in fact working. The tests for this library are also using supertest and they are able to get proof such as this:

https://github.com/expressjs/compression/blob/b9c63ced82b9f719cd5d9fd250c8432b00752d89/test/compression.js#L437

What could I be doing to not get this result?

Here's what my generated response headers look like:

{ 'x-powered-by': 'Express',
  'content-type': 'text/html; charset=utf-8',
  'content-length': '26',
  etag: 'W/"1a-9wdSANFCOffJEltF7qG/8w"',
  vary: 'Accept-Encoding',
  date: 'Sat, 14 Jan 2017 01:59:21 GMT',
  connection: 'close' }
dougwilson commented 7 years ago

What does startServer() do? Does just combining together the code above also not show the Content-Encoding? If it does work, would probably need to understand more code (and probably version of this module / version of Node.js / what other versions you think are relevant).

dougwilson commented 7 years ago

Also can't tell what handleRender() does.

Remember that this module does check the content-length and the content-type to decide compression, so make sure you're meeting those checks. If you run your tests with DEBUG=compression that should hopefully print out if compression is actually happening in this module or not.

faceyspacey commented 7 years ago

start server does the code in server.js, just wrapped in a function for use by the tests.

...I'm seeing the tests set the threshold to 0. I'm gonna try that now.

faceyspacey commented 7 years ago

...i think it must be the content-length! ..since I'm mocking handleRender() and it returns a very small response.

dougwilson commented 7 years ago

Ah, I see you're editing the original post still. I see in there that the content-length is 26, which is well below the default of 1kb (https://github.com/expressjs/compression#threshold) that is needed to trigger compression. You can always set the threshold to 0 for testing if necessary as well, or send back a bigger response.

faceyspacey commented 7 years ago

...im running into some other issues with my own codebase, but I'm gonna close this cuz I'm 99% sure threshold: 0 will solve it and because your timeliness is much appreciated. thanks again!

dougwilson commented 7 years ago

Cool, feel free to comment here / reopen if that doesn't turn out the case and we can troubleshoot.

faceyspacey commented 7 years ago

I got it to work, but I found a new weird issue. I'm getting the appropriate content-encoding if I do this:

res.status(200)
res.setHeader('Content-Type', 'text/plain')
//res.end('hello world')

but not if I uncomment the final line:

res.status(200)
res.setHeader('Content-Type', 'text/plain')
res.end('hello world') // res.send('hello world') also does not work

what I'm getting in the test is this:

request(server).get(path)
      .set('Accept-Encoding', 'gzip')
      .end((err, res) => {
          typeof res === 'undefined' // here's what I'm getting

The threshold is 0, but I can't actually send anything without it breaking down.

dougwilson commented 7 years ago

Hm, that is weird. Normally if you don't include res.end or res.send, your server will actually just hang and never respond. If you are commenting that out and your actually getting a server response, something else has to be calling res.end or res.send in your code. Are you able to reproduce with the only contents of your app being the following?

var compression = require('compression')
var express = require('express')

var app = express()

app.use(compression({ threshold: 0 }))

app.use(function (req, res) {
  res.status(200)
  res.setHeader('Content-Type', 'text/plain')
  res.end('hello world')
})

module.exports = app
faceyspacey commented 7 years ago

I found the issue:

serverSideRender(req, res)
   .catch(error => console.log('SSR ERROR', error))

vs:

serverSideRender(req, res) // this works!

serverSideRender is my mocked function where I was calling res.end('hello world'). If I didn't use it as a promise, it works. I'm not sure what I make of that..

dougwilson commented 7 years ago

Definitely weird... not sure without understand what's inside that function :)