API Bug: infinite loop possible with plugin settings

オープン
#358 コメント 3 件 リアクション 0 件 担当者 1 名 GitHub で見る

@EduardMe がすでに取り組んでいます。

2022年11月7日 から。

評価

この issue はまだ評価されていません。

説明

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

主要言語
JavaScript
スター
204
フォーク
82
平均マージ
22時間 27分
マージ済み PR(30日)
3

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

NotePlan/plugins のほかの issue

NotePlan/plugins の issue をすべて見る

似ている issue

JavaScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。