Skip to content

STJ-23: BUGFIX: deprecation_tracker breaking with unknown keywords #157 #158

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

julioalucero
Copy link

STJ-23: BUGFIX: deprecation_tracker breaking with unknown keywords

Description

BUGFIX: Prevent DeprecationTracker from crashing when receiving unknown keyword arguments.

In certain environments (e.g. when using sass-embedded), the warn method receives unexpected keyword arguments such as :deprecation, :span, or :stack. Previously, these unrecognized keywords would raise an ArgumentError and interrupt execution. This patch safely handles unknown keyword arguments while preserving known ones.

Motivation and Context

Fixes #152 — DeprecationTracker was breaking during setup in projects using sass-embedded due to unhandled keyword arguments in warn.

This update ensures that warn forwards all known and unknown keyword arguments safely to super, and gracefully falls back to known keywords if the parent method does not accept extras.

How Has This Been Tested?

  • Added a test to verify that warn can accept unknown keyword arguments (e.g. :deprecation, :span, :stack) without raising errors.
  • Ran the full test suite using Ruby 2.7 and Ruby 3.2 to ensure backward and forward compatibility.
  • Verified that the warn method continues to capture and store deprecation messages as expected.

Screenshots:

I will abide by the code of conduct

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@julioalucero looks good to me 👍🏼

(spoke too soon)

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

Once you've addressed my one comment, please add an entry to the CHANGELOG

super(*messages, **keyword_args, **kwargs)
rescue ArgumentError
super(*messages, **keyword_args)
end
Copy link
Member

Choose a reason for hiding this comment

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

@julioalucero if you know when the signature of the method changed, it would be best to use if/else vs. begin/rescue

@julioalucero julioalucero marked this pull request as ready for review May 29, 2025 18:11
@julioalucero
Copy link
Author

@etagwerker, adding the changelog like this is ok? 99576c2

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@julioalucero You can add a line like this one to the CHANGELOG and we should be all good to go: 3d35c23#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR4

@julioalucero
Copy link
Author

You can add a line like this one to the CHANGELOG and we should be all good to go: 3d35c23#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR4

@etagwerker added! Thanks for the review!

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.

[BUG] NextRails deprecation_tracker breaking with unknown keywords:
2 participants