ここは日本一クールでエッジなモダンフロントエンド開発が行われている現場。前後左右どこを向いても、優秀なエンジニアしかいません。
そんな環境で1年ほど働いているうちにまるで自身も優秀なエンジニアであるかのような錯覚に陥ってしまったわたしは、完全に調子こいてました。
事故は得てして、そんな時に起こるものです。
https://youtu.be/DO-aAUKd2yo
バグがほら潜んでるよ こんなにある
調子乗ってやってると本番が落ちる
事故がほら 起きるよ
いい気になってると
よく「運転に慣れた頃が一番事故を起こしやすい」と言いますが、これは何も車やバイクに限った話ではありません。
事故る奴は不運(ハードラック)と踊(ダンス)っちまった?
いいえ、シンプルに慢心です。
というわけで、フロントエンドエンジニアを名乗り出して3年になる人間が披露するにはちょっと恥ずかしい、ごく初歩的なミスによるやらかしネタではありますが、懺悔と供養ということで公開しちゃおうと思います。
何をやらかしたのか
一言で説明すると、非同期のループ処理をミスって自サービスに DoS 攻撃を食らわせてしまいました。
もうちょっと詳しく
その時わたしは、自サイトのページのうち条件に合致するURLが何件あるかを調査するため、 Node.js で cli を作っていました。
(ちなみにこの cli の中身の話も、余裕ができたら書きたいなと思っています。余裕ができたら…)
cli が行なっている処理をざっくり説明すると、
- Search Console API を叩いて第一条件に合致するURLを取得する
- 取得してきたURLの一覧を非同期のループ処理で順番にスクレイピングして第二条件に合致するものを抽出
- その結果を外部ファイルに出力する
というものになります。
わかる人はもうピンときているかなと思いますが、わたしがやらかしたのはこの「非同期のループ処理で順番にスクレイピング」というところで、要は非同期でループ処理をする時のお作法をちゃんと守っていなかったために、瞬間的に本番環境に大量アクセスを発生させちゃったわけです。
今回のやらかしを深く反省して、async function と反復処理について振り返ってみます。
async function と反復処理
複数の非同期処理を実行する場合、 async function は for ループなどの反復処理と組み合わせることができます。
では早速やってみましょう。
事前準備として Promise を返す関数を準備する
以下の関数を3回実行する方法を考えてみましょう。
// 1000ミリ秒後に Date.now() - time でresolve される関数
async function count(time: number) {
return new Promise((resolve) =>
setTimeout(() => resolve(Date.now() - time), 1000)
);
}
case1: 愚直にやっていき
駆け出しエンジニアだった頃の自分を特殊召喚してコードを書いてもらいました。
async function case1() {
console.log("---start---");
const time = Date.now();
console.log(await count(time));
console.log(await count(time));
console.log(await count(time));
console.log("---end---");
}
いやはや、なんともJS1年生らしい、フレッシュで初々しいコードですね!
実行結果はこんな感じになりました。
ミリ秒単位の微妙なズレはまあ、そういうもんです。
---start---
1005
2011
3015
---end---
case2: for文で反復処理
続いて今回の失敗を踏まえて反省したタイミングの自分を召喚してコードを書いてもらいます。
async function case2() {
console.log("---start---");
const time = Date.now();
for (let i = 0; i < 3; i++) {
console.log(await count(time));
}
console.log("---end---");
}
まあ、そうですよねって感じのコードですね。
実行結果はこんな感じになりました。
---start---
1003
2008
3013
---end---
case3: forEach で反復処理
では次のコードはどうでしょうか?
async function case3() {
console.log("---start---");
// [0, 1, 2] という配列を作る
const range = Array.from({ length: 3 }, (v, i) => i);
const time = Date.now();
range.forEach(async () => {
console.log(await count(time));
});
console.log("---end---");
}
実はこれ、やらかしをした瞬間の自分を召喚して書いてもらったコードです。( わざわざ [0, 1, 2]
という配列を作って forEach してるのは例としてわかりやすくするためのものであり、今回のやらかしコードとはちょっと異なりますが)
わかる人はすでに色々言いたいことがありすぎてムズムズしてるかと思いますが、まあグッと堪えてください。
実行結果はこうなります。おっと?思ったのと違いますね?
---start---
---end---
1005
1006
1006
case3 の関数は何がダメだったの?
わたしは錯乱した
やらかした直後で焦っているわたしは愚かにもこう思ったのでした。
「あれ、でも、 forEach のコールバック関数は async function で、その中に await 式があるわけだから、 console.log(await count(time))
が実行されるのを待ってループの次のカウントに行くんじゃないの?」
async function case3() {
console.log("---start---");
const range = Array.from({ length: 3 }, (v, i) => i);
const time = Date.now();
// コールバックが async function だから
range.forEach(async () => {
// この await の処理を待って次のループに行くのでは?
console.log(await count(time));
});
console.log("---end---");
}
信頼と実績の MDN 様の記述を見てみよう
await 式は async function の実行を一時停止し、Promise の解決または拒否を待ちます。解決した後に async function の実行を再開します。再開するときに await 式は解決された Promise にラップされた値を返します。
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Operators/await
つまり async function の中で await を使うと、実行が一時停止されるわけですね。
それを踏まえてもう一度例のコードを見てみましょう。
async function case3() {
console.log("---start---");
const range = Array.from({ length: 3 }, (v, i) => i);
const time = Date.now();
// コールバック関数の async function は、コールバック関数の中の await 式の実行を待つ
range.forEach(async () => {
console.log(await count(time));
// なので例えばここに
// console.log("hoge") とか書いてあったら
// count(time) の実行後に ”hoge" が出力されるわけですね
});
console.log("---end---");
}
このコードの何がまずかったか、だんだん見えてきましたね。
コールバックって何なの
コールバック関数のasync function は、コールバック関数の中の await の実行を待ちます。
case3 で forEach のコールバックとして記述されている関数は、 await count(time)
以降に何の処理も書かれていないので、 await count(time)
の実行を待って行われる処理は何もないわけです。
さて、 forEach という関数は、配列のそれぞれの要素に対して、登録されたコールバック関数を1度だけ実行します。
これがどういうことかというとつまり……
// こんなコードがあったとして
[0, 1, 2].forEach(async () => {
console.log(await count(time));
});
// それは以下を実行しているのと同じことだったってわけ
(async () => {
console.log(await count(time));
})();
(async () => {
console.log(await count(time));
})();
(async () => {
console.log(await count(time));
})();
こう書き換えてみると、そりゃダメだって感じしますよね。
信頼と実績の MDN 様再び
わたしのようなド低脳の阿呆がこんなしょうもないミスで自サービスに DoS 攻撃を食らわせたりすることがないように、forEach のページにちゃーんと注意書きがありました。
forEach は同期関数を期待する
forEach はプロミスを待ちません。forEach のコールバックとしてプロミス (または非同期関数) を使用する場合は、その意味合いを理解しておくようにしてください。
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach
大事なことは全てドキュメントに書いてあります。大人は「そんなの知らなかった」じゃ済まされないのです。
同じ過ちを繰り返さないために
人間は愚かなので、「次は気を付けようね」で済ませたら絶対にまた同じことをやらかします。
こういうのはなるべく仕組みで防げるようにしておきたいですね。
本番実行前に小さくテストする
「本番環境」に「ループで」スクレイピングを行うという行為に対して、あまりに危機感を欠いていました。
幸い今回は大丈夫でしたが、これがもし無限ループ系のやらかしだったら…………
ループによるスクレイピングは慎重に。
事前に以下のようなテストを実行しておけば、ログの内容から「あれ?何かおかしいな?」と気付けたかもしれません。
async function case3_test() {
console.log("---start---");
const range = Array.from({ length: 100 }, (v, i) => i);
const time = Date.now();
// ループの外にカウンタを設置しておく
let cnt = 0;
range.forEach(async () => {
// ループの5回目以降は return される
if (cnt >= 5) {
return;
}
cnt++;
// 以下のコードは最初の5回しか実行されない
console.log(await count(time));
});
console.log("---end---");
}
case3_2();
業務で作ったツールは全てリポジトリ化して共有スペースに置くことをルール化する
実はこのツール、レビューなしで動かしちゃったんです。
もちろんプロダクションコードであればそんなことは絶対に許されません。
しかし今回の惨劇の原因となったこのツールは、わたしが自身のタスクをちょっと楽にするために作った、共有する予定もない、調査タスクが終わったら破棄するつもりのコードだったので、「まあいいか、みんなにレビューの時間使わせるのも悪いしな」という意識が働いてしまったのでした。
いや、反省してます。レビューなしとか本当にありえないですよね。
完全に調子こいてました。
使い捨て前提のツールだったとはいえ、業務の一環で作ったものは全て社内ツールです。であればそれは、チームメンバー全員がアクセス可能な場所に置かれているべきでした。
そこに置いてけば防げたか?それでも結局ちゃんとレビュー依頼するの忘れたら意味ないのでは?というご指摘はごもっともですが、オープンにしておくことで誰かに見てもらえる可能性はぐっと高まりますよね。
そんなコードで大丈夫か?
ありがとうございました!