supertest icon indicating copy to clipboard operation
supertest copied to clipboard

Error response: WebSockets request was expected

Open nickgrealy opened this issue 3 years ago • 6 comments

I have an almost identical test case to https://github.com/visionmedia/supertest/issues/566 , except in my test I'm iterating over the await request().get().expect() 1000 times, to test the performance of the endpoint.

80% of the time, the tests pass, but occasionally I hit the following issue.

Error: expected '{"mydata":0}' response body, got 'WebSockets request was expected\r\n'

(400 Bad Request)

Has anyone seen this before?

I've tried adding Promise awaits for the handlers in the beforeEach/afterEach, to no avail.


Node: v18.3.0 Express: 4.17.1 Supertest: 6.2.4

TestCase.ts

import express, { Application } from 'express'
import { Server } from 'http'
import { describe, it } from 'mocha'
import request from 'supertest'
import MyRouter from '../../src/routes/MyRouter'

describe('wip PerformanceTesting - 1000x should be less than X milliseconds', () => {
    const app: Application = express().use(MyRouter)
    let server: Server

    beforeEach(async () => {
        server = await new Promise((resolve) => {
            const s = app.listen(0, () => resolve(s))
        })
    })

    afterEach(async () => {
        await new Promise((resolve) => server.close(() => resolve(null)))
    })

    it('my performance test', async () => {
        for (let i = 0; i < 1000; i++) await request(app).get('/api/sales/data').expect('{"mydata":0}').expect(200)
    }).timeout(1000)
})

nickgrealy avatar Sep 07 '22 16:09 nickgrealy

Makes me wonder whether this issue has always been possible, but just that there's a low likelihood of it occurring.

Related: https://github.com/davidje13/superwstest/issues/11

( @davidje13 - you might be interested? )

nickgrealy avatar Sep 08 '22 00:09 nickgrealy

I suspect this is a different issue to the one I raised (#566), but you do also have that issue (since you are using request(app) instead of request(server), so it doesn't matter that you're managing your own server.

If you are seeing "WebSockets request was expected", that means a server has successfully started on that port already and is serving responses, but the server is expecting websocket requests. Looking around for that error message, it seems to be related to the built-in debugger (launched by passing --inspect-brk to the node invocation. I'd guess maybe your IDE is doing this automatically?)

My guess is that in the event that request(app) randomly picks the same port that the node debugger is already using, you get this error. It's a bit surprising though, because I think request(app) uses 0 for the port, which is supposed to pick a random available port.

Replacing request(app) with request(server) may fix both issues. I guess it would be interesting to see if it doesn't fix the specific issue you see here though (if it doesn't, that could actually be a bug in NodeJS itself)

davidje13 avatar Sep 08 '22 21:09 davidje13

Thinking about this some more, I think you are getting the port conflict due to differing hosts:

  • --inspect-brk defaults to listening on 127.0.0.1:9229 (ref)
  • thing.listen(0) defaults to listening on an available port on :: or 0.0.0.0 (and this is what request(app) does under-the-hood when called with an express app, but also what you're doing in your explicit code)

Since the hosts are different, I think the latter will consider 9229 to be an available port, and when it is chosen you will see this issue (because you end up with more than one server on port 9229; one listening just locally on your machine, and the other listening on the network too). You are seeing this particularly often in your test because as-written, you are actually starting 1,000 concurrent servers in the test, each needing its own port (but with the fix I mentioned above, you would only start a single server, as you intended).

So to fully fix it, you should do the fix I mentioned in my comment above and also update your app.listen(0, ...) line to app.listen(0, '127.0.0.1', ...) (or use '::1' if you prefer IPv6). This is a good thing to do anyway since it means your test server won't be visible on the local network, which is unlikely to be what you want.

davidje13 avatar Sep 08 '22 22:09 davidje13