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

fix issue #15 #16

Merged
merged 2 commits into from
Jan 14, 2016
Merged

fix issue #15 #16

merged 2 commits into from
Jan 14, 2016

Conversation

revington
Copy link

This commit aims to fix issue #15
Explanation:
The wrapper does not handle properly a socket that close before any write.

I know the code formatting of file test/fix-issue-15 is not the convention you are following in this project.
I will reformat the file if you explain to me what are your preferences.

@cusspvz
Copy link
Member

cusspvz commented Jan 13, 2016

Hmm. Now I'm getting, this is happening only when you're not in strict mode and send an empty request, so seems that who are in strict mode isn't affected (because it would fail).

while (null != (chunk = socket.read())) {
chunk = socket.read();

if(null === chunk && header.length === 0){
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add here a non-strict condition filter?
! options.strict && null === chunk && header.length === 0

Copy link
Member

Choose a reason for hiding this comment

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

And also run your tests to check if this affects somehow your fix? Cheers

@revington
Copy link
Author

Problem arises on both scenarios. I have updated the test to cover both scenarios.
Code does not need to be updated.

cusspvz pushed a commit that referenced this pull request Jan 14, 2016
@cusspvz cusspvz merged commit 47f6501 into mosano-eu:master Jan 14, 2016
@revington revington mentioned this pull request Jan 15, 2016
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