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 the node-fetch cloned response hanging bug as best I can #580

Merged
merged 3 commits into from
Jul 19, 2020

Conversation

wheresrhys
Copy link
Owner

While this is technically a breaking change - responses returned by inspected calls won't always be readable in the way they were before - I feel it's necessary to roll back the hanging bug for users of v9. Prioritising the users of this new feature (only released about a month ago) over established users who may come across this troubling bug #575 feels like poor prioritisation.

The solution I've gone with is to clone responses on demand in the inspection stage. If this fails (because the response has already been read), it falls back to overwriting the json, text etc methods of the original response with functions that return the promises generated by those original function calls, which are captured by the proxy wrapped around the original response.

A better solution making more use of the proxy feels possible, along the lines of lazily cloning only at the point that the original body is used. That feels more robust, but also hard work to implement properly, so want to get this fix out first.

@coveralls
Copy link

coveralls commented Jul 19, 2020

Coverage Status

Coverage increased (+0.04%) to 96.4% when pulling e0b574f on nasty-clone-bug into 684a0b4 on master.

@wheresrhys wheresrhys linked an issue Jul 19, 2020 that may be closed by this pull request
@wheresrhys wheresrhys merged commit b9b0b03 into master Jul 19, 2020
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.

Call log entries may cause response body reading to hang
2 participants