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

Improve error message for syntax error in cassette #909

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

sambostock
Copy link
Contributor

When deserializing a cassette and encountering a syntax error, the error message can be confusing. This is exacerbated in projects using Rails' backtrace silencer, as the error has no apparent source outside the test itself.

One common scenario resulting in syntax errors is the use of ERB in the cassette, but omitting the :erb option to use_cassette.

For example, a cassette containing may contain a JSON snippet for an API call where a field is populated using

"field": <%= "some computed value".to_json %>

If given erb: true, this will work fine, but if this is forgotten, a syntax error will be encountered.

This change appends to the error's message, indicating that the error was encountered reading a VCR cassette, and suggesting the use of the :erb option.

When deserializing a cassette and encountering a syntax error, the error
message can be confusing. This is exacerbated in projects using Rails'
backtrace silencer, as the error has no apparent source outside the test
itself.

One common scenario resulting in syntax errors is the use of ERB in the
cassette, but omitting the `:erb` option to `use_cassette`.

For example, a cassette containing may contain a JSON snippet for an API
call where a field is populated using

    "field": <%= "some computed value".to_json %>

If given `erb: true`, this will work fine, but if this is forgotten, a
syntax error will be encountered.

This change appends to the error's message, indicating that the error
was encountered reading a VCR cassette, and suggesting the use of the
`:erb` option.

# @private
ENCODING_ERRORS = [ArgumentError]

# @private
SYNTAX_ERRORS = [::Psych::SyntaxError]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my tests, this was always the error I encountered, regardless of if I was using Psych or not.

I'm assuming this is because under the hood Syck is gone, and Psych is still used?

My concerns are

  • on a version of Ruby prior to the introduction of Psych, will this blow up if the constant doesn't exist?
  • on a version of Ruby with both Syck and Psych, will this simply not work?

I'm not intimately familiar with the subtleties of Ruby's YAML library history though, so ideally someone who is can chime in, as I may be misunderstanding what's going on.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't worry about that, our earliest-supported version is Ruby 2.5.

How I reasoned: In the notes at the end of the docs, https://ruby-doc.org/stdlib-2.5.3/libdoc/yaml/rdoc/YAML.html it's mentioned that Syck was the predecessor, and that Psych is the "implementation".

def handle_syntax_errors
yield
rescue *self::SYNTAX_ERRORS => e
e.message << "\nNote: This is a VCR cassette. If it is using ERB, you may have forgotten to pass the `:erb` option to `use_cassette`."
Copy link
Member

Choose a reason for hiding this comment

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

I had to go and look if it << was alright to use with an exception, and it IS

$ RUBYOPT=--enable-frozen_string_literal pry
09:31:52 >> e = RuntimeError.new
=> #<RuntimeError: RuntimeError>
09:32:02 >> e.message = "hi"
NoMethodError: undefined method `message=' for #<RuntimeError: RuntimeError>
Did you mean?  message
from (pry):3:in `__pry__'
09:32:07 >> e.message
=> "RuntimeError"
09:32:24 >> e.message << "e"
=> "RuntimeErrore"

Yeah, versus << on other Strings:

09:31:45 >> "" << "olle"
FrozenError: can't modify frozen String: ""
from (pry):1:in `__pry__'

So, thanks!

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

@sambostock Thanks! This is a valuable developer-experience improvement!

@olleolleolle olleolleolle merged commit b1d5526 into vcr:master Oct 27, 2021
@sambostock sambostock deleted the cassette-erb-syntax-errors branch October 27, 2021 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.

None yet

2 participants