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

Hotfix: Catch Throwable instead of Exception in LockingMiddleware #160

Open
wants to merge 1 commit into
base: 1.x
Choose a base branch
from

Conversation

javespi
Copy link

@javespi javespi commented Apr 16, 2024

Hello 👋

If for instance a ValueError is thrown in a command handler using this locking middleware will execute second command.

I discovered this because new enum feature from PHP released on 8.1 throws a ValueError exception when value is not expected calling from method.

enum Foo: string
{
    case BAR = 'bar';
}

Foo::from("not-bar"); // throws ValueError exception

Some playground with enums and exceptions:

Then, ValueError extends Error which implements Throwable but does not extends Exception so catch is not working as we expect.

I did the test with Error class since it is available from PHP 7.

On a side note, I realized this LockingMiddleware is not available in master branch but was not released 2.0 yet, so I patch 1.x which I see has recent commits.

@javespi
Copy link
Author

javespi commented Apr 30, 2024

@rosstuck hey 👋
whenever you have the chance and time, could you please take a look to this hotfix? It will enable compatibility with PHP Enums feature 🙏

Thank you so much!

@javespi
Copy link
Author

javespi commented Jul 10, 2024

Hello @frankdejonge @snapshotpl! Is there any chance you guys can take a look to this PR? 🙏

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.

1 participant