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 error generation bug. #16

Closed
wants to merge 1 commit into from
Closed

Fix error generation bug. #16

wants to merge 1 commit into from

Conversation

jkearse3
Copy link

@jkearse3 jkearse3 commented Jun 14, 2016

I had an issue in relation to this one ( #12 ) and I think I may have found the cause. Turns out having the request created prior to the for loop where tr.RoundTrip is called is forcing the connection to cancel (see line 1402 of https://golang.org/src/net/http/transport.go where pc.t.replaceReqCanceler is called, and subsequently it's function definition on line 642). The comments on replaceReqCanceler says:

// replaceReqCanceler replaces an existing cancel function. If there is no cancel function
// for the request, we don't set the function and return false.
// Since CancelRequest will clear the canceler, we can use the return value to detect if
// the request was canceled since the last setReqCancel call.

That means that the request outside of the loop is in turn getting canceled due to replaceReqCanceler function call that is with in *transport.RoundTrip returning false if I understand this correctly.

Creating a new request per RoundTrip seems to solve this error generation. So I moved the request preparation inside the loop. Is this okay? Would love help with making sure I'm looking in the right direction.

EDIT:
Disregard for now, I think I'm going on the wrong path here. Quite confused.

@jkearse3 jkearse3 changed the title Fixed error generation bug. Fix error generation bug. Jun 14, 2016
@elmacnifico
Copy link
Contributor

let me know when you got somewhere

@jkearse3 jkearse3 closed this Jun 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