Load content to buffer before attempting deserialization

[LichP]

What kind of change does this PR introduce?

Fixes issue #1384 by ensuring response content is loaded to a buffer before attempting serialization.

What is the current behavior?

When an exception is thrown during deserialization, ApiException attempts to capture the request content to the Content property to make it available when the exception is being handled (e.g. so apps can log the content to aid debugging). However, the underlying stream in the HttpContent object has already been consumed by the deserialization and cannot be re-read.

What is the new behavior?

By loading the content to a buffer before attempting to deserialize we ensure the stream can be re-read by ApiException.

What might this PR break?

I've moved the LoadIntoBufferAsync call to just before the call to the (de)serializer to ensure we only do this on this exact code path, rather than as soon as the content variable is set as I originally suggested in #1384. This should avoid any problems that could arise from API calls that expect to handle the HttpContent or Stream directly (calling LoadIntoBufferAsync is probably not desirable for these scenarios where we might expect endpoints to return long-lived connections or very large payloads).

Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:

[ChrisPulman]

Unfortunately the tests are falling with this addition, please could you see how this may be resolved.

[LichP]

The failing test was for the non-seekable stream case: to address this I'm now obtaining the underlying stream with ReadAsStreamAsync and only attempting the call to LoadIntoBufferAsync if the stream is seekable. I've also adjusted the ThrowOnGetLengthMemoryStream mock to appropriately implement the CanSeek property.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.63%. Comparing base (6ebeda5) to head (1660837).
Report is 28 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1705      +/-   ##
==========================================
- Coverage   87.73%   87.63%   -0.11%     
==========================================
  Files          33       33              
  Lines        2348     2329      -19     
  Branches      294      290       -4     
==========================================
- Hits         2060     2041      -19     
- Misses        208      210       +2     
+ Partials       80       78       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.