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.

Sign up for GitHub

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

Jump to bottom

Fix issue #4501 Exception noise from OAuth and REST (API2 ) #4642

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

Fix issue #4501 Exception noise from OAuth and REST (API2 ) #4642

wants to merge 2 commits into from
+30 −3

Conversation

kiatng
Copy link
Contributor

@kiatng kiatng commented Feb 22, 2025

Description (*)

See issue #4501. This PR added $shouldLog as an optional parameter in Mage_APi2_Exception, any code that extends it and overrides the constructor with the old signature would break.

Related Pull Requests

  • see OpenMage/magento-lts#<issue_number>

Fixed Issues (if relevant)

@github-actions github-actions bot added the Component: Api2 Relates to Mage_Api2 label Feb 22, 2025
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@sreichel
Copy link
Contributor

any code that extends it and overrides the constructor with the old signature would break.

#4501 (comment)

How about to add _criticalNotLoggable() method and a new exception?

@kiatng
Copy link
Contributor Author

kiatng commented Feb 28, 2025

any code that extends it and overrides the constructor with the old signature would break.

#4501 (comment)

How about to add _criticalNotLoggable() method and a new exception?

I have thought about new exception class. I am hesitant because I do not think there is any custom code that would extend it, if there is, it's easy to fix. This avoid adding more code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Component: Api2 Relates to Mage_Api2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exception noise from OAuth and REST (API2 )
2 participants
@kiatng @sreichel