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 the messaging in our unidentified action exceptions. #1060

Merged
merged 1 commit into from
May 22, 2024

Conversation

barryhughes
Copy link
Member

@barryhughes barryhughes commented May 21, 2024

We trigger InvalidArgumentExceptions from a number of locations, all with the same basic message:

Unidentified action <ID>

This is not particularly helpful, and conveys no information about what was happening at the time the exception was thrown. This change adds some additional context to a number of these exception messages.

Updates #1059 (but does not close).


Testing instructions

This PR changes the text of various exception messages, but there are no functional changes to the way the library works. Manual testing will therefore have limited value, but I've still provided some basic instructions to see the change in action.

✍🏼 Note that you need to have access to the system/PHP error log, or some other way to view errors logged via error_log() calls:

  1. Open Tools ‣ Scheduled Actions
  2. Open the very same screen in a second browser tab.
  3. Identify and check the box for the same action in both browser tabs.
  4. Using the bulk actions, delete the action first in one tab and then the other.
  5. You should see something similar to the following being logged:
Action Scheduler was unable to delete action 12345. Reason: Unidentified action 12345: we were unable to delete this action. It may may have been deleted by another process.

If that seems a little verbose, note that the first half (everything before Reason:) is added by the List Table implementation when it catches the exception.

Changelog

Fix - Provide more context when throwing InvalidArgumentException exceptions.

@barryhughes barryhughes marked this pull request as ready for review May 21, 2024 23:20
@barryhughes barryhughes requested review from a team and jorgeatorres and removed request for a team May 21, 2024 23:20
Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @barryhughes!

@jorgeatorres jorgeatorres merged commit 6360e7d into trunk May 22, 2024
46 checks passed
@jorgeatorres jorgeatorres deleted the fix/1059-unidentified-action-exceptions branch May 22, 2024 18:03
@barryhughes barryhughes added this to the 3.8.1 milestone May 22, 2024
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.

2 participants