Skip to content

Consume fetch response body on error#468

Merged
sarangj merged 1 commit into
qafrom
consume_response_body
Sep 25, 2025
Merged

Consume fetch response body on error#468
sarangj merged 1 commit into
qafrom
consume_response_body

Conversation

@sarangj

@sarangj sarangj commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Doing some digging, I read that a potential source of memory leaks is not consuming the response body of a fetch response. Noticed that we don't consume the body on error, so changed things up a bit to parse and log the text of the body if we don't get an ok response.

Did a bit of refactoring as a part of this, namely, getting rid of the nested error handling and removing the 422 specific handling, which probably made sense when this was first written but now is probably fine to remove since we have a stabilized API.

Ticket:

This PR does the following:

Open questions

How has this been tested? How should a reviewer test this?

Accessibility concerns or updates

Checklist:

  • I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.
  • I have updated the CHANGELOG.md.

@vercel

vercel Bot commented Sep 25, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
digital-collections Ready Ready Preview Comment Sep 25, 2025 5:23pm

Doing some digging, I read that a potential source of memory leaks is
not consuming the response body of a fetch response. Noticed that we
don't consume the body on error, so changed things up a bit to parse and
log the text of the body if we don't get an ok response.

Did a bit of refactoring as a part of this, namely, getting rid of the
nested error handling and removing the 422 specific handling, which
probably made sense when this was first written but now is probably fine
to remove since we have a stabilized API.
@sarangj sarangj force-pushed the consume_response_body branch from 4d7b706 to 17b0ba9 Compare September 25, 2025 17:21
@sarangj sarangj merged commit fe77893 into qa Sep 25, 2025
5 checks passed
@sarangj sarangj deleted the consume_response_body branch September 25, 2025 17:29
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