Agent executes "end()" callback before setting cookies

[vsuhanov]

when using agent to keep persistent cookies it executes end callback before agent saves cookies and the next request might not have these cookies set.

here is an example of test that fails when it should not.
https://gist.github.com/vsuhanov/8599800

possible solution:

1. try to insert event handler, that saves cookies, in the beginning of the event handleк list — unfortunately default eventEmitter of node doesn't support it and it might be not such a great idea to rearrange the list.
2. change the way(the place where) the callback is executed. Right now request class attaches an on('response') listeneу when it is created if I move it to Request.prototype.end method all tests pass but they yield "double callback!" so I assume it's wrong.

[hallas]

can you provide a runnable example where it fails?

[vsuhanov]

just put this file in a test/node folder. Or how want me to do it?

https://gist.github.com/vsuhanov/8665978

[hallas]

would be really great if it was something I could just copy paste and run with node :) nvm ill fix it my self

[veeti]

Ping. Just spent a hour trying to figure out why this wasn't working just to find this issue.

[gjohnson]

Hmm... I have not really messed with the agent additions much. Maybe @hunterloftis would have a thought or two before I dig in.

[rubenv]

Oh what a crappy bug, lost a lot of time on this one. While I don't have a patch for it, here's a way to work around it:

agent
    .post('/queue/4/status')                                                                                                                  
    .expect(200, status)
    .end(function (err) {
        process.nextTick(function () {                                                                                                        
            cb(err);                                                                                                                          
        });                                                                                                                                   
    });  

Running the callback in the next tick ensures that the next request will only happen once the cookies are set.

[vsuhanov]

I made a pull request #354 with a possible solution.

But it fails travis build on node version 0.8 which appears to be a general issue right now I noticeв that all latest builds crash on it.

[madhums]

any updates on this?

[alsotang]

seems no. I think my fix is ok:

alsotang@bb47cd4

[ralyodio]

it still doesn't work for me.

[madhums]

@alsotang the tests are failing #366.

[madhums]

Oh nevermind, its in node 0.8

[alsotang]

@madhums okok, forget 0.8

[mocheng]

Hit by this bug.
It looks there is already quite a few PR available. Not merged yet?

[iammaart]

+1
This bug is still present in v0.21.0.

[Dynasty9]

+1 I've lost days to this.

[ilanbiala]

Has this been merged in and released?

[defunctzombie]

I don't think it has. I will take a look this week.

[alexanderlperez]

+1

[alexhung]

Just lost like 4 hours on this. +1 on merging and release soon.

[defunctzombie]

The PR needs to be reworked. I think we need a cleaner way to do it than the way the PR is doing it.

[elbuo8]

👍 to this.

[livin]

How it's possible that such an import bug is not yet fixed. Why this testing library is needed if it can't handle cookies at all. Struggled with it for enough time. Now thinking about replacing controller testing with something else. Perhaps e2e testing instead.

[defunctzombie]

Fixed by 04a04e2