[REQUEST] Detect and report infinite loops / recursive or self-referential data loading

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

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
25/100
issue の種類
機能追加
明瞭さ
おおむね明確
活発さ
停滞
技術スタック
javascript, node.js
領域
backend

調査の方向性

2つの例に示されている DataLoader.load のフローと batch 関数の動作から始めます。自己参照 load と、同じ batch 内の依存関係のケースをそれぞれ再現し、その後、通常のロード動作を変更せずにそれらを報告する方法を検討します。これらのハングによって、問題のある load 関係の特定に役立つ有用な警告またはエラーが生成されれば完了です。

索引モデルが issue の本文から書いたものです。

説明

enhancement

If you accidentally create a self-referential DataLoader load, the loader silently creates a loop of promises which never resolve. This is obviously a programmer error, but it can be pretty hard to diagnose when things just silently don't work, if the recursive load isn't obvious across a complex codepath.

This code demonstrates the issue:

import DataLoader from 'dataloader';

const dumbDataLoader = new DataLoader(
  async nums => await Promise.all(nums.map(async n => await dumbDataLoader.load(n)))
);

console.log(await dumbDataLoader.load(3))

I'd expect to get some sort of warning or error from DataLoader to try to pin down the issue, but instead I only get this warning from Node (which in practise won't pop up for a more complex server):

Warning: Detected unsettled top-level await at index.js:7
console.log(await doubleDataLoader.load(3))

Worse, a variation of this can pop up even when the loads are technically not infinitely recursive, since you can have one batch waiting on a result from the same batch. Consider this code:

import DataLoader from 'dataloader';

const factDataLoader = new DataLoader(
  async nums => await Promise.all(nums.map(async n => {
    if (n == 0) return 1;
    return n * (await factDataLoader.load(n - 1));
  }))
);

// This works:
console.log(await factDataLoader.load(0));
console.log(await factDataLoader.load(1));

factDataLoader.clearAll();

// This does not:
console.log(await Promise.all([
  factDataLoader.load(0),
  factDataLoader.load(1),
]));

That isn't actually infinitely recursive, but because 0 and 1 end up in the same batch, and 1 depends on 0, and the result of 0 can't get to the computation around 1 because they're waiting on the same batch... this ends up hanging too. This is basically the same issue, but it's really hard to debug!

This example might look contrived, but this hit me in real-world code. My data loaders do data fetching and complex privacy checks. I have arbitrarily-nested posts, and you can only see a post if you can load the post's parent. If you try to load a post and its parent in the same batch of a data loader, it hangs without any error or warning!

主要言語
JavaScript
スター
13.4k
フォーク
522
PR マージ指標
30日以内にマージされた PR はありません

環境構築

はじめの一歩

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

graphql/dataloader のほかの issue

graphql/dataloader の issue をすべて見る

似ている issue

JavaScript の issue をもっと見る

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

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