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

test: test case for issue #1120 #1121

Closed
wants to merge 6 commits into from
Closed

test: test case for issue #1120 #1121

wants to merge 6 commits into from

Conversation

PopGoesTheWza
Copy link
Contributor

Sample test to reproduce issue #1120

Checklist

  • I have read the documentation.
  • I have included a pull request description of my changes.
  • I have included some tests.
  • If it's a new feature, I have included documentation updates.

@PopGoesTheWza
Copy link
Contributor Author

@szmarczak @sindresorhus please comment this (very) kludgy fix.

In short, .buffer(), .json() and .text() shortcuts consume the body Buffer bound to the initial asPromise() call and cannot "see" the other body bound to the asPromise() which is called during the afterResponse hook.

There is very likely a cleaner way to handle this issue.

source/as-promise.ts Outdated Show resolved Hide resolved
@PopGoesTheWza
Copy link
Contributor Author

@szmarczak here is a cleaner fix (type casting is somewhat ugly though)

@szmarczak
Copy link
Collaborator

If you don't mind waiting until I finish up the Rewrite PR, it will be much easier to look into this later...

@PopGoesTheWza
Copy link
Contributor Author

Sure.

@sindresorhus
Copy link
Owner

This can continue now. #1051 was merged.

@PopGoesTheWza
Copy link
Contributor Author

Let me now if and how I may assist

@szmarczak
Copy link
Collaborator

Thanks for the reproducible example, fixed in 99d70df

@szmarczak szmarczak closed this Apr 15, 2020
@PopGoesTheWza PopGoesTheWza deleted the issue-1120-test branch April 23, 2020 06:54
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.

None yet

3 participants