Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable request.jar #21

Closed
wants to merge 1 commit into from
Closed

Disable request.jar #21

wants to merge 1 commit into from

Conversation

kesla
Copy link

@kesla kesla commented Jan 13, 2012

Request has since 2.9.0 introduced support for cookie jars.

It means that if a Cookie-header is sent without using the cookie jar it will be overwritten by request.

Disabling the cookie jar functionality in request makes api-easy work like it used to. Tests passes when using an older version of request (2.2.9) also.

@indexzero
Copy link
Contributor

@kesla Looks good. Can we make this configurable somehow?

@kesla
Copy link
Author

kesla commented Jan 18, 2012

@indexzero If we want to make it configurable (and we should) we probably want to use this.options as suggested in https://github.com/flatiron/api-easy/blob/master/lib/api-easy.js#L99 and use options.request to override any of request's default values.

That way you could, for example, configure api-easy to disable it cookie jar and not follow redirects like this:

suite.discuss('When using your awesome API')
       .discuss('and your awesome resource')
       .use('localhost', 8080, {
         'request': {
           'followRedirect': false,
           'jar': false
         }
       })
       .setHeader('Content-Type', 'application/json')
       .post({ test: 'data' })
         .expect(200, { ok: true })
         .expect('should respond with x-test-header', function (err, res, body) {
           assert.include(res.headers, 'x-test-header');
         })
       .export(module);

Sounds reasonable? Should I close this pull request and open a new one with the suggested api above?

@indexzero
Copy link
Contributor

@kesla Yes. Sorry for the long-turn around here. Please re-open a new pull-request

@indexzero indexzero closed this Jun 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants