Supertest: double callback! warning

[benmkramer]

I noticed I was getting a double callback warning in quite a few of my tests using supertest.

I found the issue was with how I was using expect and end as follows

request(app)
  .get('/something')
  .expect(200, function (res) {
    // some logic
    return false
  })
  .end(done);

Changing that to the following resolved the issue for me

request(app)
  .get('/something')
  .expect(function (res) {
    // some logic
    return false;
  })
  .expect(200, done);

To see the double callback warning, see this example

Note that this is using express 4.4.1 and supertest 0.13.0

[benmkramer]

Looking back at this, I think it was just a misunderstanding on my part, where I thought fn in expect(status[, fn]) could be a custom assertion function, but instead it was treating that as the actual callback for the given test.

[gjohnson]

@benmkramer that is correct, expect(Function) is for custom assertions and expect([Arguments], [Function]) is for request/test callbacks. It's confusing I know.

Leaving this open as a documentation reminder.

[hex7c0]

Hi,
the new release throw this issue when using HEAD method only

request(app).head('/').expect('Content-Type', /json/).expect(400, done);
no problem with 0.13.0, or with 0.14.0 and GET o POST methods

[nielskrijger]

The new 1.1 release released a couple of hours ago broke our tests by throwing this error. Not sure why yet, the "usual suspects" I've seen mentioned within the issue tracker didn't seem to apply. For now downgraded supertest (deadlines, you know ;-) ).

Most likely it has to do with Supertest's upgrade to superagent 1.3.0

If I find out what the issue was I'll let you know. For now this is just a heads-up for others out there experiencing the same issue with 1.1.

[nielskrijger]

Found the cause and a fix.

As it turns out superagent 1.3 now supports promises and if you were calling supertest within the context of a promise but supertest itself isn't promisified it will all mess up.

The following showcases the problem and a fix (coffeescript);

    fit 'RUNS SUCCESSFULLY', (done) =>
      userId = UserModel.id DEV_USER._id
      request app
      .post '/api/login'
      .send { username: DEV_USER.email, password: DEV_PASSWORD }
      .expect 200
      .end done

    fit 'RUNS SUCCESSFULLY', (done) =>
      userId = UserModel.id DEV_USER._id
      request app
      .post '/api/login'
      .send { username: DEV_USER.email, password: DEV_PASSWORD }
      .expect 200
      .end (err, res) ->
        UserModel.update userId, { changePassword: true }
        .then (result) =>
          done()

    fit 'THROWS DOUBLE CALLBACK WARNING', (done) =>
      userId = UserModel.id DEV_USER._id
      UserModel.update userId, { changePassword: true }
      .then (result) =>
        console.log 'This is printed to console', result # Result is a valid JSON object
        request app
        .post '/api/login'
        .send { username: DEV_USER.email, password: DEV_PASSWORD }
        .expect 200
        .end (err, res) ->
          console.log 'This is never printed to console'
          done() # Never called
      .catch (err) ->
        console.log 'This is printed to console', err # err is "TypeError: first argument must be a string or Buffer]"
        done.fail()

    fit 'FIX, AS IT TURNS OUT SUPERAGENT 1.3 NOW SUPPORTS PROMISES', (done) =>
      userId = UserModel.id DEV_USER._id
      UserModel.update userId, { changePassword: true }
      .then (result) =>
        console.log 'This is printed to console', result # Result is a valid JSON object
        request app
        .post '/api/login'
        .send { username: DEV_USER.email, password: DEV_PASSWORD }
        .expect 200
      .then (err, res) ->
        console.log 'This is printed to console'
        done()

[light24bulbs]

This seems like a bug to me. Why can't we have a callback within the context of a promise? Especially since the errors this situation produces are cryptic.

[n0m0r3pa1n]

I think I get an error connected with this question. Here is my test:

it 'should login user', ->
    createUser("gmirchev90", "1234")
    request(sails.hooks.http.app)
      .post("/api/login")
      .send({username: "gmirchev90", password: "1234"})
      .end (err, res) ->
        console.log(err);
// I get an exception:
// TypeError: First argument must be a string or Buffer

but if I change it with:

request(sails.hooks.http.app)
      .post("/api/login")
      .send({username: "gmirchev90", password: "1234"})
      .expect(200)

Now everything works. Even if I set a wrong code like 400 I get an exception that I got 200 instead of 400. Is this connected with this issue?

[nielskrijger]

@n0m0r3pa1n I think it is. I got similar errors while working on this issue.

Your test it 'should login user', -> does not have a done render the test a promise most likely (if you're using mocha at least).

My guess is it will work with callbacks as well if you change your test to this:

it 'should login user', (done) ->
    createUser("gmirchev90", "1234")
    request(sails.hooks.http.app)
      .post("/api/login")
      .send({username: "gmirchev90", password: "1234"})
      .end (err, res) ->
        done(err)

Basically avoid any callbacks end (err, res) when using promises. They don't play nice in supertest and superagent 1.3+.

From what I've read superagent 2.0 supports Promises fully rather than their "Faux Promise" implementation in 1.3+
https://github.com/visionmedia/superagent/releases

Currently supertest latest version bump of superagent was in its 1.2.0 release to superagent ^1.7.2 https://github.com/visionmedia/supertest/releases

For our project we decided to stick to supertest 1.1.0 until superagent 2.0 is supported, I don't like the "Faux Promise" concept in Superagent 1.3.

[blacksonic]

Just need to remove the .end() call from the request and handle the returned Promise.

This should be documented, really confusing with co-supertest.
https://github.com/avbel/co-supertest/issues/9

[kumar303]

Just need to remove the .end() call from the request and handle the returned Promise.

How do you shut down the server then? As I understand from https://github.com/visionmedia/supertest/issues/100 .end() is required to shut down the server.

[gbersac]

Had the problem :

let app = express();
app.get("/", (req, res) => {
    res.send("Hello world !"); // send static value
});

it("sould return hello world response", (done) => {
    supertest(app)
        .get("/")
        .expect(200, (err: any, res: Response) => {
            expect(res.body).to.equal({ error: "Page not found" });
        });
       .end(done);
});

To solve the issue, I just had to remove the .end(done).