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

Allow agent reuse for ClientSSLSecurity if forever option is set #974

Merged
merged 1 commit into from
Oct 2, 2017

Conversation

Mik13
Copy link
Contributor

@Mik13 Mik13 commented Oct 2, 2017

It should be possible for SSL connections to reuse the tls session. Every request, the https agent is recreated.

This PR changes this:
If the forever flag is set in the options per request (3rd parameter) or by default when setting the security (4th parameter), it should reuse the same agent as the previous requests also using forever.

The request-Module handles everything else, like setting the keep-alive-flag to the agent and the https module will cache the tls-session to the agent. Because of this, it is important that the same agent is used.

Tests added

@coveralls
Copy link

coveralls commented Oct 2, 2017

Coverage Status

Coverage increased (+0.02%) to 93.536% when pulling c8492b1 on Mik13:forever into 6a7a49a on vpulim:master.

@herom
Copy link
Contributor

herom commented Oct 2, 2017

thanks a lot @Mik13 - could you please document the feature in our README.md file too? 👍

if (!this.agent) {
options.keepAlive = true;

this.agent = new https.Agent(options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please initialize this property also within the constructor-function

firstOptions.agent.should.not.equal(secondOptions.agent);
});

it('should not generate different agents if paramter given', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the description to "should return the same agent if paramter is present"

@Mik13
Copy link
Contributor Author

Mik13 commented Oct 2, 2017

You're welcome @herom. I've changed the test description and added a short documentation to the readme. I hope this works

@coveralls
Copy link

coveralls commented Oct 2, 2017

Coverage Status

Coverage increased (+0.03%) to 93.539% when pulling faf498e on Mik13:forever into 6a7a49a on vpulim:master.

@herom
Copy link
Contributor

herom commented Oct 2, 2017

Awesome 💛 Thanks a ton @Mik13 😸

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.

3 participants