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

Preserve original exceptions in parameters #341

Merged
merged 1 commit into from
Jun 12, 2018
Merged

Conversation

enumag
Copy link
Member

@enumag enumag commented Jun 7, 2018

Note: This is a quick edit using GitHub editor. I didn't really check or test it yet.

}

if ($event->getParam('concurrencyException', false)) {
throw new ConcurrencyException();
throw $event->getParam('concurrencyException');
Copy link
Member

Choose a reason for hiding this comment

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

why get twice? just do if ($exception = $event->getParam('concurrencyException', false)) {

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really like assignments in ifs but sure, I'll change it.

@enumag
Copy link
Member Author

enumag commented Jun 11, 2018

@prolic Done. The coding style failure is not related.

@enumag
Copy link
Member Author

enumag commented Jun 11, 2018

There isn't much I can do in tests for this change - the exceptions didn't really change, except for their their callstack and I don't think we want to test that.

Also I briefly tested it on my project along with pdo-event-store 1.9.0. It works as expected. The error I struggled with would be much easier to debug now.

@prolic
Copy link
Member

prolic commented Jun 11, 2018

@enumag Can you also update TransactionalActionEventEmitterEventStore ?

@enumag
Copy link
Member Author

enumag commented Jun 11, 2018

Oh, I missed a class again lol. Sure, I'll update it tomorrow.

@prolic
Copy link
Member

prolic commented Jun 11, 2018 via email

@enumag
Copy link
Member Author

enumag commented Jun 12, 2018

@prolic Done.

@prolic prolic merged commit 0db5504 into prooph:master Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants