NotePlan / NotePlan/plugins

API Bug: infinite loop possible with plugin settings

Aperta
#358 3 commenti 0 reazioni 1 assegnatario Vedi su GitHub

@EduardMe ci sta già lavorando.

Dal 7/11/2022.

API bug
Lingua principale
JavaScript
Stelle
204
Fork
82
Merge medio
22h 27m
PR unite (30g)
3

Descrizione

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

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.