Skip to content

"breaking change" in json 2.9.1->2.10.0? #754

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

Closed
bastelfreak opened this issue Feb 22, 2025 · 2 comments
Closed

"breaking change" in json 2.9.1->2.10.0? #754

bastelfreak opened this issue Feb 22, 2025 · 2 comments

Comments

@bastelfreak
Copy link

Hi,
I'm one of the maintainers of https://github.com/voxpupuli/metadata-json-lint, where we validate a JSON file for the Puppet ecosystem. The relevant code (I think so, at least):

  # https://github.com/voxpupuli/metadata-json-lint/blob/03574807f8a7e05cfc22b48a86c25fd2137b5d75/lib/metadata_json_lint.rb#L103C1-L107C8
    begin
      parsed = JSON.parse(f)
    rescue Exception => e
      abort("Error: Unable to parse metadata.json: #{e.exception}")
    end

We've some tests in the module, they pass with json 2.9.1 and older. One of the tests assume the following error:

Error: Unable to parse metadata.json: unexpected token at '{
  "name": "puppetlabs-postgres'

With json 2.10.1 we get:

Error: Unable to parse metadata.json: expected ',' or '}' after object value, got: '"version": "3.4.1",
  "author": '

This could be considered as a breaking change, if we assume the output is even something like a stable/public API. Can you tell me if the rewording was intentional?

@byroot
Copy link
Member

byroot commented Feb 23, 2025

Yes it was intentional: #729

if we assume the output is even something like a stable/public API

It isn't. The exception class is, but the exception message is intended for humans and can change in any version. Heck, up until I fixed it a couple years ago, it was changing on every versions (#470). They're also not consistent between different implementations of the gem (Java, Pure, etc).

Also I have further plans to improve these messages, like including line a column numbers, so it might break again.

So it's probably best if your test only assume a specific error class, but doesn't try to match the message content.

bastelfreak added a commit to voxpupuli/metadata-json-lint that referenced this issue Feb 23, 2025
json 2.10.0 updated the text in exceptions. This was done in
ruby/json#754. metadata-json-lint prints the
exception and our test verified the exception text. That's no longer
consistent between oder and newer json messages.

I think it provides no benefit to match the exception text, so lets
remove it from our test.
@bastelfreak
Copy link
Author

Thanks for clarification. I just removing matching for the exception text in our tests.

bastelfreak added a commit to voxpupuli/metadata-json-lint that referenced this issue Feb 23, 2025
json 2.10.0 updated the text in exceptions. This was done in
ruby/json#754. metadata-json-lint prints the
exception and our test verified the exception text. That's no longer
consistent between older and newer json messages.

I think it provides no benefit to match the exception text, so lets
remove it from our test.
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

No branches or pull requests

2 participants