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

Set the request.Body to http.NoBody if request.Body is nil #105

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

ilijamt
Copy link
Contributor

@ilijamt ilijamt commented Oct 11, 2024

I noticed that on certain requests I get a missing interaction error. But looking at the yaml file the interaction is there.
I tracked it down to certain Post requests that do no have any body. So when the replay matches the body is nil, and r.ParseForm returns an error when it shouldn't.

https://github.com/dnaeon/go-vcr/blob/e544a7e468e5bd51652002be6cfd6ee604f945b0/pkg/cassette/cassette.go#L345C1-L352C1

and the error is raised

https://cs.opensource.google/go/go/+/refs/tags/go1.23.2:src/net/http/request.go;l=1272-1276;drc=29b1a6765fb5f124171d94f157b6d6c3b2687468

An example of what causes the error to occur.

https://github.com/ilijamt/vault-plugin-secrets-gitlab/blob/33f02606e7871715fb682d08b8a7232e40720eab/testdata/fixtures/16.11.6/TestWithServiceAccountUser.yaml

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 52.29%. Comparing base (e544a7e) to head (a237db7).

Files with missing lines Patch % Lines
pkg/cassette/cassette.go 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##               v4     #105      +/-   ##
==========================================
- Coverage   52.50%   52.29%   -0.21%     
==========================================
  Files           4        4              
  Lines         499      501       +2     
==========================================
  Hits          262      262              
- Misses        217      219       +2     
  Partials       20       20              

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

@dnaeon
Copy link
Owner

dnaeon commented Oct 11, 2024

Thanks!

@dnaeon dnaeon merged commit e0eabf6 into dnaeon:v4 Oct 11, 2024
2 checks passed
@ilijamt ilijamt deleted the form-body-nil-fix branch October 13, 2024 20:30
@colinodell
Copy link

colinodell commented Nov 27, 2024

Do you know when a new version will be tagged with this fix? It's the only thing blocking me from upgrading to v4.

[edit] As a temporary solution I think I can just decorate the default matcher with the behavior from this PR:

matcher := func(r *http.Request, i cassette.Request) bool {
	if r.Body == nil {
		r.Body = http.NoBody
	}

	return cassette.DefaultMatcher(r, i)
}

@dnaeon
Copy link
Owner

dnaeon commented Nov 27, 2024

Just tagged it https://github.com/dnaeon/go-vcr/releases/tag/v4.0.2

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.

4 participants