1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

Partial Fix Buggy add-on can stop mass toggling add-ons

Discussion in 'Resolved Bug Reports' started by Jon W, Jul 14, 2015.

  1. Jon W

    Jon W Well-Known Member

    Not entirely sure if this is a suggestion or a bug, but I suspect bug.

    If an add-on throws/returns an error when an attempt is made to disable it, this could halt the mass toggle add-ons process half way, having only disabled the add-ons that appear in the list before this buggy add-on.

    This is made worse by refreshing the page, which causes the process to re-run using the current list of enabled add-ons, which means the original cached list of enabled add-ons is now gone.

    There might also be legitimate reasons for an add-on to show a warning or throw/return an error if it is disabled. For example, it might be that an add-on stops itself from being disabled if it knows that disabling itself would cause serious errors. For example (although it doesn't actually do this), an add-on like NoForo by Waindigo (which deletes core database tables) might stop itself from being disabled.

    Perhaps the mass toggle code could be included in a try/catch block and/or the XenForo_DataWriter::ERROR_SILENT error handler could be enabled.

    Thanks. :)
    semprot and Mike Creuzer like this.
  2. Mike

    Mike XenForo Developer Staff Member

    I've adjusted this as far as possible. There's a separate preSave loop and the save is wrapped in a transaction.

    However, I would note that there isn't really an expectation that toggling the active state of an add-on is something that a) an add-on would involve itself with, or b) that an add-on can prevent. If disabling an add-on would cause serious errors, that add-on really shouldn't be approached like that. If other add-ons depend on that add-on and it's not available, they should behave "defensively" and ensure that they get don't errors (by checking for the other add-on in the add-on cache).

    I'm tagging this as a partial fix because it's certainly possible for an add-on to do something that will break the defensive code we have added (especially if it's executing DDL queries), but I would suggest if that happens, the add-on should be reapproached.
    Chris D likes this.

Share This Page