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

invoke cancel function after response has been fully read #370

Closed
wants to merge 1 commit into from

Conversation

varmalho-eg
Copy link

Context

We were using [email protected] version of okta sdk and using ListUsers and few other APIs. However, those APIs started returning no response after upgrade to [email protected] version of SDK.

Summary

When caller passes timeout parameter for these APIs, sdk invokes context cancel function before response has been fully read causing truncation of response. This was not getting caught as io.ReadAll error return was getting ignored. context cancel function needs to be called after all io.ReadAll invocations on http response body.

ref: https://groups.google.com/g/golang-nuts/c/2FKwG6oEvos

I believe this issue #363 is also related to this same bug.

Go Version: 1.19.3
Os Version: MacOS 12.4
OpenAPI Spec Version:

Context:
We were using [email protected] version of okta sdk and using ListUsers and
few other APIs. However, those APIs started returning no response
after upgrade to [email protected] version of SDK.

Summary of the issue:
When caller passes timeout parameter for these APIs, sdk invokes
context cancel function before response has been fully read causing
truncation of response. This was not getting caught as io.ReadAll
error return was getting ignored. context cancel function needs to
be called after all io.ReadAll invocations on http response body.

ref: https://groups.google.com/g/golang-nuts/c/2FKwG6oEvos
@varmalho-eg
Copy link
Author

varmalho-eg commented Apr 6, 2023

Ah, my friend already posted a PR -- closing this in favor of #369

@varmalho-eg varmalho-eg closed this Apr 6, 2023
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