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.

Sign up for GitHub

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

Jump to bottom

Fix net/http adapter and an issue with nil responses #23

Merged
merged 5 commits into from
Jan 21, 2025
Merged

Fix net/http adapter and an issue with nil responses #23

merged 5 commits into from
Jan 21, 2025

Conversation

SSDany
Copy link
Contributor

@SSDany SSDany commented Jan 15, 2025

This PR fixes 2 issues:

  1. Prepending Net::HTTP with NetHttpAdapterPatch may cause errors if another tool (e.g. faraday) passes a block to Net::HTTP request.
  2. In some cases (good examples are invalid params, especially invalid queries for GraphQL, or just raise from an action) we will have a nil response in process_action.action_controller event payload. I think we can just treat it as an empty response, using 0 as body size.

DmitryTsepelev
DmitryTsepelev reviewed Jan 16, 2025
View reviewed changes
Copy link
Owner

@DmitryTsepelev DmitryTsepelev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, could you please make specs pass? :)

@DmitryTsepelev DmitryTsepelev merged commit 3c68d80 into DmitryTsepelev:main Jan 21, 2025
1 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DmitryTsepelev DmitryTsepelev DmitryTsepelev left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SSDany @DmitryTsepelev