rshdhere commented on PR #6633: URL: https://github.com/apache/fineract/pull/6633#issuecomment-6532311801
Thanks @galovics, good catch. You're right: every command that opts in through `HookableCommand` would lose the `status: Exception` notifications the legacy path sends today. The dispatcher already calls `CommandHookError` hooks in its catch block, so this should only need a `WebhookCommandHookError` next to the after-hook. It would: - turn the throwable into an `ErrorInfo` with `ErrorHandler`, the way `SynchronousCommandProcessingService` does, so the `response` (`errorCode`, `statusCode`, the parsed error message) matches the legacy one - publish straight away rather than after commit, since a failed command doesn't commit, which is also what `publishHookErrorEvent` does today - share the payload building with the after-hook so the two can't drift Since this one is already approved and green, I'd rather keep it scoped and do the error hook as a follow-up. I'll open a JIRA ticket and link it here. If you'd prefer it in this PR, I can add it here instead. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
