Skip to content

fix(languages/analyzer): TypeError on null committer#21

Open
ferferga wants to merge 1 commit intogh-metrics:masterfrom
ferferga:fix-null-committer
Open

fix(languages/analyzer): TypeError on null committer#21
ferferga wants to merge 1 commit intogh-metrics:masterfrom
ferferga:fix-null-committer

Conversation

@ferferga
Copy link

First of all, thank you very much for trying to keep this project active.

In the old version (and in this one), I'm receiving this error at the rendering stage:

TypeError: Cannot destructure property 'committer' of 'undefined' as it is undefined.
    at file:///metrics/source/plugins/languages/analyzer/recent.mjs:70:21
    at Array.filter (<anonymous>)
    at RecentAnalyzer.patches (file:///metrics/source/plugins/languages/analyzer/recent.mjs:70:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async RecentAnalyzer.analyze (file:///metrics/source/plugins/languages/analyzer/recent.mjs:25:21)
    at async file:///metrics/source/plugins/languages/analyzer/recent.mjs:19:7
    at async results (file:///metrics/source/plugins/languages/analyzer/analyzer.mjs:64:7)
Node.js v20.20.0

This pull request fixes the issue.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a crash in the languages “recently-used” analyzer when processing PushEvent commit entries that are undefined or have missing committer data.

Changes:

  • Hardened PushEvent commit filtering to avoid destructuring committer from an undefined commit.
  • Added an explicit guard for commit.committer.email before applying the authoring filter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

commits
.flatMap(({payload}) => payload.commits)
.filter(({committer}) => filters.text(committer?.email, this.authoring, {debug: false}))
.filter(commit => commit?.committer?.email && filters.text(commit.committer.email, this.authoring, {debug: false}))
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new filter still uses filters.text(...), which returns true when the value is not in the patterns list. Given commits_authoring is documented/used elsewhere as the set of identifiers to match (e.g., code plugin filters commits where author matches an entry), this likely inverts the intent and can exclude the user’s own commits when commit.committer.email is present. Consider aligning with the other commit-authoring filters by treating commits_authoring as an inclusion match (or explicitly negating filters.text if you want exact-match semantics).

Suggested change
.filter(commit => commit?.committer?.email && filters.text(commit.committer.email, this.authoring, {debug: false}))
.filter(commit => commit?.committer?.email && !filters.text(commit.committer.email, this.authoring, {debug: false}))

Copilot uses AI. Check for mistakes.
commits
.flatMap(({payload}) => payload.commits)
.filter(({committer}) => filters.text(committer?.email, this.authoring, {debug: false}))
.filter(commit => commit?.committer?.email && filters.text(commit.committer.email, this.authoring, {debug: false}))
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a runtime TypeError for malformed PushEvent commit entries, but there’s no automated coverage for the specific edge case (e.g., payload.commits containing undefined/null or commits missing committer). Since there is an existing languages plugin test case for the “recently-used” section, consider extending the GitHub activity mock to include such an entry so the regression is caught.

Copilot uses AI. Check for mistakes.
Signed-off-by: Fernando Fernández <ferferga@hotmail.com>
@lishaduck
Copy link
Member

Are they really forcing copilot reviews on everybody these days? That sucks.

This looks fine but there's like 20 equivalent PRs in the lowlighter repo and I want to attribute it correctly.
I'll leave this open as a reminder for me to do that (I'll add you as a coauthor)

@ferferga
Copy link
Author

ferferga commented Feb 3, 2026

Oops, kwoning it was unmaintained, I didn't even bother checking the original repo. I will leave the related PRs listed here for reference to future people coming across this thread, feel free to add more @lishaduck. I will also edit this comment if I find more

@lishaduck
Copy link
Member

Oops, kwoning it was unmaintained, I didn't even bother checking the original repo.

Nah, you're good! I just keep directing people over there to file over here so I'd feel bad not giving them credit.
I'll try to get them merged in tonight or tomorrow 🤞🏻, pls remind me if I forget!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants