v2.1.0 NewAsMode fails with ModeReplaying and missing cassette file

[JVecsei]

The latest minor version update contained a breaking change in the NewAsMode function.

go-vcr/v2/recorder/recorder.go

Lines 193 to 198 in 6bd2f2d

	// Check if the cassette exists
	if _, err := os.Stat(cassetteFile); os.IsNotExist(err) {
	// Replaying mode should fail if no cassette exists
	if mode == ModeReplaying {
	return nil, cassette.ErrCassetteNotFound
	}

In the previous version we were using (v.2.0.1), this function just created an empty cassette if there was no cassette file found with the given name. With v2.1.0 this behavior changed and calling NewAsMode using ModeReplaying in combination with a non-existing cassette filename returns an error.
Especially in test scenarios where the recorder is just setup in a SetupTest method for all tests, independent of whether all test methods execute an outgoing http call, this is an undesired behavior.

Could we get this function backwards compatible by introducing a flag which defines whether a missing cassette file should return an error?

Thanks in advance!

[dnaeon]

Hey @JVecsei ,

Sorry about that. Will check it shortly. The issue came from this MR, which changed the default behaviour some time ago, but was never tagged and got into v2.1.0.

#66

Will submit a fix shortly, which you can test.

[dnaeon]

Hey @JVecsei ,

I've actually took the time to clean up the package structure a bit and moved v2 package into it's separate branch, which I'm going to be making the default one soon.

Could you please test out gopkg.in/dnaeon/go-vcr.v2 ? The previous behaviour has been brought back as well.

Please pay attention that you also need to update your package imports to use the new package URL.

Let me know how it goes.

Thanks!

[JVecsei]

Hey @JVecsei ,

I've actually took the time to clean up the package structure a bit and moved v2 package into it's separate branch, which I'm going to be making the default one soon.

Could you please test out gopkg.in/dnaeon/go-vcr.v2 ? The previous behaviour has been brought back as well.

Please pay attention that you also need to update your package imports to use the new package URL.

Let me know how it goes.

Thanks!

Hey @dnaeon,

thanks for the quick response & fix!

These lines though still cause trouble on our side:

go-vcr/recorder/recorder.go

Lines 207 to 214 in c444037

	cassetteFile := fmt.Sprintf("%s.yaml", cassetteName)
	// Check if the cassette exists
	if _, err := os.Stat(cassetteFile); os.IsNotExist(err) {
	// Replaying mode should fail if no cassette exists
	if mode == ModeReplaying {
	return nil, cassette.ErrCassetteNotFound
	}

Before it was just working when passing in ModeReplaying and there was no cassette file, if there was no outgoing call made via the recorder's transport. Now it just fails with the cassette.ErrCassetteNotFound error.

[dnaeon]

Ah, I see now. I was under the impression that the default mode of operation was causing the issue for you. I've overlooked that detail :)

I do think that for ModeReplaying it is more correct that it does replaying only, while the ModeReplayingOrRecording is probably what you need.

While the previous behaviour of ModeReplaying was to replay and record, I think that current behaviour is the correct one, and previous was not.

Is it possible for you to switch to either ModeReplayingOrRecording when using recorder.NewAsMode() or simply use recorder.New() which uses ModeReplayingOrRecording from now on?

[JVecsei]

Thanks again for your quick feedback!

Since ModeReplayingOrRecording is also updating recordings with missing interactions we don't necessarily want to execute it in this mode in different scenarios.
We will try to implement a workaround then.

Thanks for your help!

[dnaeon]

Hey @JVecsei ,

I'm working on a v3 version of go-vcr and I think your issue will be covered by one of the existing modes in the new version.

I've decided to spend some time cleaning up the API, fixing the tests and add a few new features to the cassettes, which would allow for more options when creating new filters, etc.

Please check the current v3 supported modes here:

• https://github.com/dnaeon/go-vcr/blob/v3/recorder/recorder.go#L45

I believe what you are looking for is the ModeRecordOnce mode.

Please check it out and let me know if that's what you need. I'm planning to release v3 in the couple of days. Tests have been updated and extended as well, so I think with v3 you should be good.

[dnaeon]

Hey @JVecsei , just tagged v3 release of go-vcr.

Please test it out and let me know if you find any issues. I'll close this one out, as I think your use case should be covered by the default mode of the recorder.

Feel free to re-open if that's not the case.

[JVecsei]

@dnaeon ,

thank you again for your help and addressing that topic. That is exactly what we are looking for!
One thing that is now different in v3 is that it is impossible to know from the mode whether it's currently recording or not.

Before it was actually exposing that by overwriting the internal mode

go-vcr/recorder/recorder.go

Lines 216 to 218 in c444037

	// Otherwise we are in a recording mode, create new cassette and enter in recording mode
	r.cassette = cassette.New(cassetteName)
	r.mode = ModeRecording

Would it be possible by e.g. exposing the IsNew field to derive that information?

go-vcr/cassette/cassette.go

Lines 223 to 226 in 4015d92

	// IsNew specifies whether this is a newly created cassette.
	// Returns false, when the cassette was loaded from an
	// existing source, e.g. a file.
	IsNew bool `yaml:"-"`

Something like recorder.IsNewCassette()?

Just as an information why this can be interesting: We do certain cleanup steps only in case a recording was done. In case we are replaying a recording, we don't want to execute these cleanup steps. I'm sure this will be useful for other users too.

Thanks again 👍

[dnaeon]

Hey @JVecsei ,

Can you please review this one?

#76

Let me know if that's what you were asking for.

Thanks!