API Bug: infinite loop possible with plugin settings

Aberta
#358 3 comentários 0 reações 1 responsável Ver no GitHub

@EduardMe já está trabalhando nisso.

Desde 7/11/2022.

Avaliação

Esta issue ainda não foi avaliada.

Descrição

API bug

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

Linguagem predominante
JavaScript
Estrelas
204
Forks
82
Merge médio
22h 27min
PRs com merge (30d)
3

Guia de contribuição

Nenhum guia de contribuição indexado para este repositório

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de NotePlan/plugins

Todas as issues de NotePlan/plugins

Issues semelhantes

Mais issues de JavaScript

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.