fix: add nil checks in htmlURL to prevent panic by MH4GF · Pull Request #289 · masutaka/github-nippou

MH4GF · 2025-11-05T01:32:10Z

問題

github-nippou list コマンド実行時に nil pointer dereference が発生し、panic でクラッシュする問題を修正しました。

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x100df33e0]

goroutine 1 [running]:
github.com/masutaka/github-nippou/v4/lib.htmlURL(0x140002a6000)
	/Users/mh4gf/ghq/github.com/masutaka/github-nippou/lib/events.go:140 +0x1a0

原因

lib/events.go の htmlURL 関数で、イベントペイロード内の Issue/PullRequest および HTMLURL フィールドが nil の場合のチェックが不足していました。

go-github ライブラリでは、これらのフィールドは omitempty タグ付きのポインタ型として定義されており、GitHub API の仕様上 nil になる可能性があります：

type PullRequestReviewEvent struct {
    PullRequest *PullRequest `json:"pull_request,omitempty"`
    // ...
}

発生時期

私の環境では、この問題は2025年10月頃から発生し始めました。

調査の結果、最近の go-github のアップデート（v63 → v69、2025年2月10日）では、イベントタイプに関する破壊的変更は含まれていませんでした。これは潜在的にずっと存在していたバグであり、最近になって顕在化した理由として以下が考えられます：

GitHub API の仕様変更: 特定の条件下で pull_request フィールドを省略するようになった可能性
データ不整合のあるイベントの増加
私の利用環境(GitHub Enterprise)のみの可能性もあります

修正内容

全てのイベントタイプで nil チェックを追加しました：

case "PullRequestReviewEvent":
    if e := payload.(*github.PullRequestReviewEvent); e.PullRequest != nil && e.PullRequest.HTMLURL != nil {
        result = *e.PullRequest.HTMLURL
    }

テスト

修正後、問題なく動作することを確認：

$ ./github-nippou list --since-date $(date -v-1d +%Y%m%d)
# 正常に出力される

Summary by CodeRabbit

Bug Fixes
- Improved stability by adding defensive checks to prevent potential crashes when accessing event data fields.

Add nil pointer checks for Issue/PullRequest and HTMLURL fields in event payloads. Event payloads may contain nil values when resources are deleted or inaccessible, unlike fresh API responses. This fixes a panic that occurs when processing events with missing data: - Check Issue/PullRequest is not nil before accessing - Check HTMLURL is not nil before dereferencing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2025-11-05T01:32:17Z

Walkthrough

Nil-safety checks were added in the htmlURL function for all event types within lib/events.go. Each case statement now verifies that nested HTMLURL pointers are non-nil before dereferencing, preventing potential panics while preserving existing functionality.

Changes

Cohort / File(s)	Summary
Nil-safety improvements in htmlURL function `lib/events.go`	Added nil-checks before dereferencing nested HTMLURL pointers in all event type cases to guard against panics when fields are missing

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Verify nil-check guards are applied consistently across all event type cases
Confirm no logic changes beyond the defensive pointer checks
Ensure no edge cases were missed in the switch statement

Poem

🐰 A rabbit hops through pointers with care,
Adding checks for nil everywhere,
No more panics when dereferencing fields,
Safe dereferencing now gently yields!

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately describes the main change: adding nil checks in htmlURL to prevent panic, which matches the core objective of fixing a nil pointer dereference bug.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/events.go (1)

123-154: Replace htmlURL-based deduplication with event.ID for uniqueness.

The current uniq function (lines 104-119) deduplicates events using their HTML URLs as keys. When nil checks in htmlURL() fail, it returns an empty string, causing all events with missing URLs to be treated as the same event. This loses distinct valid events.

Since github.Event has an ID field (*string) in go-github v69, using event.ID as the deduplication key would be more robust and prevent data loss for events lacking HTML URLs.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7c0dff and e69fb7d.

📒 Files selected for processing (1)

lib/events.go (1 hunks)

🔇 Additional comments (1)

lib/events.go (1)

132-150: Nil checks correctly prevent panic across all event types.

The nil checks are properly implemented with consistent guarding of both parent objects (Issue/PullRequest) and their nested HTMLURL pointers before dereferencing. This solves the panic issue described in the PR objectives.

masutaka

ありがとうございます！ちょうど今週この問題に気づき、なぜ過去のリリースも panic になるんだろう？自分の環境問題？などと考えていたところでした。

masutaka · 2025-11-05T11:55:28Z

@MH4GF 🚀 v4.2.40

MH4GF · 2025-11-05T12:16:35Z

確認ありがとうございます！ライブラリのアップデートによるものじゃないのか？？と気になっていたところでした。
リリースありがとうございます！！明日からの日報がまた楽になります🙌🙌

masutaka · 2025-11-05T12:22:41Z

リリース前には毎回ローカルで動作確認しているので、ライブラリのアップデートによるものではないはず 🙃

MH4GF marked this pull request as ready for review November 5, 2025 01:35

MH4GF requested a review from masutaka as a code owner November 5, 2025 01:35

coderabbitai bot reviewed Nov 5, 2025

View reviewed changes

masutaka added the bug label Nov 5, 2025

masutaka approved these changes Nov 5, 2025

View reviewed changes

masutaka merged commit 4b54ee5 into masutaka:main Nov 5, 2025
8 checks passed

masutaka-github-nippou-ci bot mentioned this pull request Nov 1, 2025

Release for v4.2.40 #285

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add nil checks in htmlURL to prevent panic#289

fix: add nil checks in htmlURL to prevent panic#289
masutaka merged 1 commit intomasutaka:mainfrom
MH4GF:fix-nil-pointer-in-htmlurl

MH4GF commented Nov 5, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 5, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

masutaka left a comment

Uh oh!

Uh oh!

masutaka commented Nov 5, 2025

Uh oh!

MH4GF commented Nov 5, 2025

Uh oh!

masutaka commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

MH4GF commented Nov 5, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

問題

原因

発生時期

修正内容

テスト

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

masutaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

masutaka commented Nov 5, 2025

Uh oh!

MH4GF commented Nov 5, 2025

Uh oh!

masutaka commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MH4GF commented Nov 5, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 5, 2025 •

edited

Loading