NotePlan / NotePlan/plugins

API Bug: infinite loop possible with plugin settings

Offen
#358 3 Kommentare 0 Reaktionen 1 zugewiesene Person Auf GitHub ansehen

@EduardMe arbeitet bereits daran.

Seit 07.11.2022.

API bug
Vorherrschende Sprache
JavaScript
Sterne
204
Forks
82
Ø Merge
22 Std. 27 Min.
Gemergte PRs (30 T.)
3

Beschreibung

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 ...

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.