Defect #39768
closed
Call to the `controller_issues_new_after_save` hook are not protected again plugin exceptions
0%
Description
The call to the `controller_issues_new_after_save` hook in the file `app/controllers/issues_controller.rb` (around line 153), does not protect against exceptions raised in the body of the hook.
This leads to the subsequent Redmine code not being run (including a page redirect) whenever new issues are created using plugins which do not themselves protect against exceptions in their code being raised.
Can the call to this hook be wrapped in a begin/rescue block?
(Many thanks for an excellent project!)
----
For the record here are my installation details:
Note that in this case the offending plugin is `redmine_issue_checklist` which has been patched in
- [fix controller_issues_new_after_save hook stale record exception ยท stephengaito/redmine_issue_checklist@565e8df](https://github.com/stephengaito/redmine_issue_checklist/commit/565e8dfb432309a8534a03c8cbe7477660e17f6e)
----
Environment: Redmine version 5.1.1.stable Ruby version 3.2.2-p53 (2023-03-30) [aarch64-linux] Rails version 6.1.7.6 Environment production Database adapter SQLite Mailer queue ActiveJob::QueueAdapters::AsyncAdapter Mailer delivery smtp Redmine settings: Redmine theme Alternate SCM: Subversion 1.14.2 Mercurial 6.3.2 Bazaar 3.3.2 Git 2.39.2 Filesystem Redmine plugins: additional_tags 3.1.0 additionals 3.1.0 katex 0.0.1 redmine_issue_checklist 2.1.0 redmine_mermaid_macro 1.1.0 redmine_recurring_tasks 0.3.4
----
Updated by Holger Just 5 months ago
- Status changed from New to Closed
- Resolution set to Wont fix
here is no explicit error handling (and specifically no error handling which just ignores errors) for any of the hooks. This is deliberately done in order to ensure that (1) errors are noticed and (2) so that the default error handling can meaningfully handle the exception, e.g. by causing a ROLLBACK of the current transaction, to ensure that the consistency of the stored data is ensured.
If we were to introduce a general begin / rescue block around each hook call which merely logs but ultimately ignores any errors, there would be considerable risk for data consistency issues. In the general case, just ignoring any exceptions is not a good idea.
Thus, if a plugin registers a handler for a hook, it is the plugin's responsibility to handle any exceptions which may be raised by its own code. If the plugin chooses to ignore all errors in its hook, it can add an empty begin/rescue block on its own.