Project

General

Profile

Actions

Defect #39768

closed

Call to the `controller_issues_new_after_save` hook are not protected again plugin exceptions

Added by Stephen Gaito 3 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Plugin API
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Wont fix
Affected version:

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

----

Actions #1

Updated by Holger Just 3 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.

Actions

Also available in: Atom PDF