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

Close stream with defer #149

Merged
merged 4 commits into from
Jan 25, 2022

Conversation

ersonp
Copy link
Contributor

@ersonp ersonp commented Jan 19, 2022

Fixes #147

Changes:

  • Added defer to RoundTrip()

How to test this PR:
Given here : #skywire-1065

@ersonp ersonp changed the title Close sesh with defer Close stream with defer Jan 19, 2022
@ersonp ersonp force-pushed the fix/close-dmsghttp-stream-defer branch from 3ef16bf to 319be67 Compare January 19, 2022 12:25
This commit adds a way to close the stream of dmsg via a defer statement with a go routine in it inside the fun RoundTrip().

The goroutine checks if the response body is closed every one second and if it is closed then we clsoe the stream if not then we keep checking till it's closed or if context is closed.
@ersonp ersonp force-pushed the fix/close-dmsghttp-stream-defer branch from 319be67 to 4feab7f Compare January 19, 2022 12:29
This commit contains some minor linting fixes.

It also removes a test log and renames the func test used in RoundTrip() to closeStream() and adds a warn log in faliure to close stream and adds a comment detailing how we check if the response body is closed.

The todo by evanlinjin is also removed as we won't be reusing the streams as we are now closing them after every use.
@ersonp ersonp force-pushed the fix/close-dmsghttp-stream-defer branch from 44c6089 to 3f08d42 Compare January 19, 2022 13:03
This commit fixes closeStream() in dmsghttp. The error check is changed from using err strings to using errors.IS to check the error http.ErrBodyReadAfterClose. Also a new error is added to the check, io.EOF.

Now we check if there is an error and if the error is equal to either http.ErrBodyReadAfterClose or io.EOF.
This commit fixes the TestHTTPTransport_RoundTrip by removing the timeout from http clients.
@ersonp ersonp marked this pull request as ready for review January 24, 2022 05:40
@jdknives jdknives merged commit 1dfce6e into skycoin:develop Jan 25, 2022
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