NotePlan / NotePlan/plugins

API Bug: infinite loop possible with plugin settings

Ouverte
#358 3 commentaires 0 réactions 1 personne assignée Voir sur GitHub

@EduardMe y travaille déjà.

Depuis le 7/11/2022.

API bug
Langage dominant
JavaScript
Étoiles
204
Forks
82
Merge moyen
22 h 27 min
PR mergées (30 j)
3

Description

I thought I had a bright idea, Eduard. But it landed up causing an infinite loop, confirmed by looking the logs.

Aim: While developing complex new plugin commands, we often create smaller commands to test various bits, and we've taken to prefixing them with test:. These shouldn't be needed by ordinary users, unless hunting down bugs with the dev. So wouldn't it be nice to hide them from the menus and command bar unless they're in DEBUG logging mode?

So I borrowed some of @dwertheimer's helper code to produce this, which should only fire when user changes settings. It checks to see if they're not in DEBUG mode, and if so hides the test:* commands:

const pluginID = "jgclark.Summaries"

export async function onSettingsUpdated(): Promise<void> {
  try {
    logDebug(pluginID, 'starting onSettingsUpdated')

    // See if we need to hide or unhide the test: commands in this plugin, depending whether _logLevel is DEBUG or not
    // Get the commands' details
    const initialPluginJson = await getPluginJson(pluginID)
    const initialSettings = await getSettings(pluginID) ?? `{".logLevel": "INFO"}`
    // $FlowFixMe[incompatible-type]
    const logLevel = initialSettings["_logLevel"]
    logInfo('onSettingsUpdated', `Starting with _logLevel ${logLevel}`)

    let updatedPluginJson = initialPluginJson
    if (initialPluginJson) {
      const commands = updatedPluginJson['plugin.commands']
      let testCommands = commands.filter((command) => {
        const start = command.name.slice(0, 4)
        return start === 'test'
      })
      logInfo('onSettingsUpdated', `- found ${testCommands.length} test commands`)

      // WARNING: savePluginJson() causes an infinite loop!
      // WARNING: So all these lines are commented out.
      // if (logLevel === 'DEBUG') {
      //   for (let command of testCommands) {
      //     updatedPluginJson = updateJSONForFunctionNamed(updatedPluginJson, command, false)
      //   }
      //   clo(updatedPluginJson, `updatedPluginJson after unhiding:`)
      // }
      // else {
      //   for (let command of testCommands) {
      //     updatedPluginJson = updateJSONForFunctionNamed(updatedPluginJson, command, true)
      //   }
      // }
      // logDebug('onSettingsUpdated', `- before savePluginJson ...`)
      // await savePluginJson(pluginJson['plugin.id'], updatedPluginJson)  // helper that calls saveJSON()
      // logDebug('onSettingsUpdated', `  - NOT CALLED OTHERWISE AN INFINITE LOOP!`)
    }
  }
  catch (error) {
    logError('onSettingsUpdated', error.message)
  }
}

David and I think this is an NP bug. It is calling the onSettingsUpdated() function when any JSON file is written anywhere using the saveJSON method. I don't think that should be the case. Only when DataStore.settings is changed should that "hook" be called.

BTW this is an example of the potential issues implementing triggers/hooks ...

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.