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 handling of service-special when incorrect parameters are given. #583

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

Conversation

eigood
Copy link
Contributor

@eigood eigood commented May 8, 2023

When using service special, and parameters are given that do not pass validation, moqui will attempt to log this. However, doing so requires an active transaction. Service special runs this after the current transaction has completed, so the validate logic fails to run, and a NO_TRANSACTION message is printed instead, hiding the root cause of the error.

So, now we implicitly validate the service call in beforeCompletion(), so the current transaction will fail, and the root cause(incorrect parameter specification) can be shown.

When using service special, and parameters are given that do not pass
validation, moqui will attempt to log this.  However, doing so requires
an active transaction.  Service special runs this *after* the current
transaction has completed, so the validate logic fails to run, and a
NO_TRANSACTION message is printed instead, hiding the root cause of the
error.

So, now we implicitly validate the service call in beforeCompletion(),
so the current transaction will fail, and the root cause(incorrect
parameter specification) can be shown.
@acetousk
Copy link
Member

acetousk commented Aug 6, 2023

Assuming there was some testing on this, this looks good to me. @eigood Have you used this patch in production or a test environment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants