わくわく技術ブログ

プログラミング・統計・機械学習

OSSにプルリクが取り込まれた

先日、OSSに自分の書いたPull Requestが取り込まれるという実績解除(?)をしました。
別にOSS活動が偉いわけではないですが、凡人にはそれほど多い経験ではないですし今後の人生の励みにもなるかもしれないですよね。
ということで、8割が記念、2割が他の人の参考になるかもしれないという気持ちで、簡単に経緯を書いておきます。

ちなみに対象のリポジトリAzure FunctionsのNode.js用ワーカーです。

きっかけ - 仕様の不明確さに対する不安

Microsoft Azureのサービスはいくつか使ったことがあります。
その中でも、Web AppsやFunctionsは、(DB部分を除けば)ほぼ無料でWebアプリケーションをデプロイできるので重宝しています。

ただFunctionsに関しては、なにやら機能が多いらしい割に、仕様が明確でないなと感じていました。
C#はクラスがはっきりしているし、MSが推してるからドキュメントも多数揃っているけど、Node.jsで使いたいなと。JavaScrptは型が曖昧だし、ドキュメントもC#より少なく見えるので隠れた仕様とかあるのではないかと思っていました。

そんなこんなで使い方を検索していた折、Azure Functions本体がGit Hubに公開されていることを知りました。そして、別リポジトリでNode.js呼び出し用のコードも公開されていることを知りました。それが上で書いたリポジトリです。
Azure Functions系のリポジトリを整理するリポジトリもありました。

バグ発見 - コードリーディング1時間

一番知りたいなと思っていたことは「自分の書いた関数はどうやって呼び出されているのか?」でした。そこで、自分の関数を呼び出しているらしい箇所を探しました。
比較的小さなリポジトリになっていたので、FunctionLoader.tsなるファイルもその中のgetEntryPoint()というめぼしい関数もすぐに見つかりました。

function getEntryPoint(f: any, entryPoint?: string): Function {
    if (isObject(f)) {
        var obj = f;
        if (entryPoint) {
            // the module exports multiple functions
            // and an explicit entry point was named
            f = f[entryPoint];
        }
        else if (Object.keys(f).length === 1) {
            // a single named function was exported
            var name = Object.keys(f)[0];
            f = f[name];
        }
        else {
            // finally, see if there is an exported function named
            // 'run' or 'index' by convention
            f = f.run || f.index;
        }
        
        // ...略...

    return f;
}

ほー、関数だけじゃなくて、複数プロパティを抱えたオブジェクトでも動くんだ、しかも丁寧なコメント付きだぞ。
ということで、3種類の指定方法を理解しました。

特に、最後のrun()index()が呼び出されるパターンは知らなかったぞ。試してみよう(※後からドキュメント読み返したら、たしかに書いてありました。)
そうやって書いてみたところ、なぜか想定どおりに動かずエラーになるパターンに遭遇。run()を定義しているのに、そこにたどり着かないぞ…
たまたまこの前日に別の有名OSSのバグっぽい挙動に遭遇したばかりだったので、Functionsのバグを引いた可能性をすぐに考えました。
測ってはないけど、リポジトリを見つけてからここまで1時間くらいだったと思います。

原因特定と対処検討

とりあえずGitHubのIssueを探し、該当事象に関するものが無いことを確認しました。
その後は、動作確認環境の準備です。幸い難しいことはなくて、README.mdに書いてあるとおりにしたらすんなりローカルで立ち上がりました。

そしてVisual Studio Codeを使って、エラーになるコードをデバッグ実行。
JavaScriptObject.keys()は関数以外も拾ううえに、prototypeの関数を取得できないのが、自分の想定どおりに動いていない原因だと判明。
関数はprototypeに定義するのが一般的ですし、classを使う場合には勝手にそうなるので、考慮不足感がありますね。

原因がわかったところで、どう実装を変えるべきかを考えました。
ここでIssueに書いてMicrosoftの人に後は任せるという手もありましたが、原因が分かっているのと、英語で正確に伝えるよりコードで伝える方が簡単かもしれない説もあったので、自分でプルリクまで投げると決意しました。

一番気をつけたことは、互換性を崩さないこと。つまり、今までエラーになって起動できなかった例は起動できるようにするが、今まで動いていたプログラムは動きが変わらないこと。
Azure FunctionをNode.jsで使っている人が少ないとは思えませんし、少しでも破壊的変更が入れば、パッチの取り込みは慎重にならざるを得ないはずだからです。

それから、テストコードを書くことも気をつけました。
あまりテストコードの揃ったリポジトリではありませんでしたが、問題点/実現したいことが明確になり、動作保証にもなるのだから、こういう時にはテストは必須なのでしょう。
ここまで書いておいて難ですが、あまりJavaScript慣れてなくて、mock-requireって何者?? みたいになりながら書いたので勉強になりました。

IssueとPull Request、そして取り込みへ

Issueはソースコードを主体に記述しました。

  • このコードでエラーになるよ
  • 似てるけどこのコードはOKだよ
  • EntryPointのこのコードが怪しいよ! (問題のソースコードを貼り付けて、該当行の横に矢印でコメント入れる)

で、その後に修正をPull Requestとして出しました。
英語マナーがわからないので、ドキドキでした。

Issue/Pull Requestを出した後には、日々「はやく取り込まれないかなー」「意図伝わってるかな…」とほぼ毎日リポジトリを覗きました。(※書いたIssueに何か動きがあったらメール通知が来るのは知ってましたが気分的にね。) シンプルな変更なのに1週間以上待つとちょっと不安になりますが、10日目で "This looks great - thank you! And thank you for adding tests :)!!" のコメントと共に取り込んでもらえました。ヤッタネ!

簡単に不具合(っぽい挙動)を見つけられた理由

JavaScript/TypeScriptに詳しいわけでもなければ、Azure Functionsの内部実装に詳しいわけでもない自分が、すぐに不具合に気づいた理由を考えます。

1. 規模の小さいリポジトリだった

Azure Functionsというサービスはなかなかに大きいですが、コア部分はC#で書かれていて、Node.js用の実装規模は小さいです。
リポジトリにファイルが多いと、目当ての機能を探すだけでも一苦労になってしまうので、当然かもしれませんが小さなリポジトリは初心者には優しいです。

2. コードを見ながら手を動かした

不具合に気づく前に、ソースコードを見ながら「ふ~ん」と手を動かしてみました。
この 「ソースコードを見ながら」「手を動かす」 というのがポイントなんだろうなと思います。
なぜそのコードが書かれたかという意図を考えながら深く読もうとする、そして実際の動きを確認して納得する/しないというのが、コード細部の理解につながるのかなと。

業務でプログラムを書く時には、多かれ少なかれドキュメントを書いてから手を付けることが多いと思います。ドキュメントを作らないスタイルの場合は、先にしっかり話し合いをして方針共有をするでしょう。
そういう状況とは手順が逆になるので、細部に気づくことができたのだろうと思います。

古より引き継がれし、ドキュメントが存在しないシステムのバグを見つけちゃったりするのは、こういう事なんですかね。(単に色々雑なだけかも。)

まとめとこれから

Azure Functionsという自分も使うことのあるOSSプロダクトに、Pull Requestを出して取り込まれました。 小さなパッチですが、不具合発見から取り込まれるまでの一連の手続きをちゃんと業務外で行ったのは初めてだったので、緊張したり喜んだりでした。
特にバグを探そうと意気込んでソースを読んだ訳ではありませんが、手を動かしていたら自然と発見しました。コードをちゃんと読むためには書くのも必要だと思いました。

Azureに貢献するべく云々というつもりは今の所ありませんが、一回経験したことで、不具合報告やPull Request提出に対する自分の中での心理的ハードルは下がったかなと思います。
OSSを使っていて何か怪しい動きに気づいたら、これからも何か行動できそうです。

余談ですが、途中にも書いた通りNode.jsはあまり使わないですし、フロント側もあまり知らないし、もうちょっとJavaScript強くなりたいですね。サーバー側をガリガリに書きたいときには、別言語を選択すると思いますが。