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

HttpCompletionOption.ResponseHeadersRead with GetJsonAsync<T> = Stream cannot be read exception #794

Closed
rwasef1830 opened this issue Dec 31, 2023 · 5 comments
Labels

Comments

@rwasef1830
Copy link
Contributor

rwasef1830 commented Dec 31, 2023

Hello,

Flurl.Http: 4.0.0, default system.text.json deserializer, .net 7.0:

Using the GetJsonAsync convenience helper with HttpCompletionOption.ResponseHeadersRead gives an exception when trying to use automatic decompression. The exception is thrown from DeflateStream (Stream cannot be read). In debugger Stream.CanRead is false.

The helper is not properly explicitly reading the response when the response is not yet loaded.

Example API: https://openexchangerates.org/api/
(needs API Key)

@rwasef1830 rwasef1830 added the bug label Dec 31, 2023
@rwasef1830 rwasef1830 changed the title HttpCompletionOption.ResponseHeadersRead with GetJson = Stream cannot be read exception HttpCompletionOption.ResponseHeadersRead with GetJsonAsync<T> = Stream cannot be read exception Dec 31, 2023
@tmenier
Copy link
Owner

tmenier commented Jan 10, 2024

ResponseContentRead is the default in Flurl so I don't believe that's related to the exception. Have a look at #474, I think that's more likely the issue here.

@rwasef1830
Copy link
Contributor Author

rwasef1830 commented Jan 10, 2024

@tmenier The same request to the same URL works correctly if I use ResponseContentRead, but I don't wish to buffer the entire response in memory, so I use ResponseHeadersRead which then produces the exception.

The same request with ResponseHeadersRead if I change the code to GetStreamAsync and then manually deserialize it, it works fine, and that's how I worked around it. I think this is definitely not deliberate, so it's probably a bug, because it was working correctly with same code in Flurl 3.0 (which defaulted to ResponseContentRead it seems).

I'll take a stab at it at the next opportunity, I think it maybe fixable with small changes only.

@tmenier
Copy link
Owner

tmenier commented Jan 10, 2024

I think a minimal example would help. Please provide a small sample that works in Flurl 3 and not in Flurl 4. I can't think of what might have changed in this area.

@rwasef1830
Copy link
Contributor Author

rwasef1830 commented Jan 10, 2024

@tmenier I just realized i mistyped in the original issue report and wrote ResponseContentRead instead of ResponseHeadersRead, that's why I got confused by your response. I apologize. I will provide a PR with unit test and will try to fix it as well. I updated the original message.

rwasef1830 added a commit to rwasef1830/Flurl that referenced this issue Jan 10, 2024
rwasef1830 added a commit to rwasef1830/Flurl that referenced this issue Jan 10, 2024
@rwasef1830
Copy link
Contributor Author

rwasef1830 commented Jan 10, 2024

Added PR with fix #800

rwasef1830 added a commit to rwasef1830/Flurl that referenced this issue Jan 10, 2024
tmenier added a commit that referenced this issue Jan 15, 2024
@tmenier tmenier reopened this Jan 17, 2024
@tmenier tmenier added this to Backlog in Default Project via automation Jan 17, 2024
@tmenier tmenier moved this from Backlog to Ready in Default Project Jan 17, 2024
@tmenier tmenier closed this as completed Jan 17, 2024
Default Project automation moved this from Ready to Released Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Default Project
  
Released
Development

No branches or pull requests

2 participants