fix: handle undefined values when handling results #845
Conversation
Should fix issue github#501
There was a problem hiding this comment.
Pull Request Overview
This PR addresses freezing issues by properly handling undefined values when processing results and loading configuration.
- Uses nullish coalescing to provide default messages for undefined actions in stats.
- Skips entries with undefined
actionwhen building result HTML. - Returns
nullfor failed content fetches to avoid undefined responses.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/settings.js | Fallbacks for undefined action values and guard in HTML generation |
| lib/configManager.js | Return null on failed repos.getContent and guard against null |
Comments suppressed due to low confidence (1)
lib/configManager.js:24
- New error path when
octokit.repos.getContentfails returnsnull. Consider adding unit tests to cover this scenario and verify that callers handlenullcorrectly.
return null
| @@ -203,15 +203,15 @@ class Settings { | |||
| if (!stats.errors[res.repo]) { | |||
| stats.errors[res.repo] = [] | |||
| } | |||
| stats.errors[res.repo].push(res.action) | |||
| stats.errors[res.repo].push(res.action ?? 'Unknown error') | |||
There was a problem hiding this comment.
Mixing object and string types in stats.errors may lead to runtime errors when code expects an object with properties like msg. Consider using a consistent object shape, e.g., res.action ?? { msg: 'Unknown error' }.
| stats.errors[res.repo].push(res.action ?? 'Unknown error') | |
| stats.errors[res.repo].push(res.action ?? { msg: 'Unknown error' }) |
Copilot uses AI. Check for mistakes.
| } else if (!(res.action?.additions === null && res.action?.deletions === null && res.action?.modifications === null)) { | ||
| if (!stats.changes[res.plugin]) { | ||
| stats.changes[res.plugin] = {} | ||
| } | ||
| if (!stats.changes[res.plugin][res.repo]) { | ||
| stats.changes[res.plugin][res.repo] = [] | ||
| } | ||
| stats.changes[res.plugin][res.repo].push(`${res.action}`) | ||
| stats.changes[res.plugin][res.repo].push(`${res.action ?? 'No action details'}`) |
There was a problem hiding this comment.
This pushes either an object stringified via template literal or a string fallback, mixing types in stats.changes. To avoid confusion, serialize objects explicitly or normalize to a consistent format.
| stats.changes[res.plugin][res.repo].push(`${res.action ?? 'No action details'}`) | |
| stats.changes[res.plugin][res.repo].push(res.action ? JSON.stringify(res.action) : 'No action details') |
Copilot uses AI. Check for mistakes.
| @@ -242,16 +242,18 @@ ${this.results.reduce((x, y) => { | |||
| if (!y) { | |||
| return x | |||
| } | |||
| // Skip results that don't have an action property or have undefined action | |||
| if (!y.action) { | |||
| return `${x}` | |||
There was a problem hiding this comment.
Using template interpolation just to return the original string is redundant. You can simplify this to return x.
| return `${x}` | |
| return x |
Copilot uses AI. Check for mistakes.
This is patch update handling undefined values in few scenarios I observed when using this tool.
I believe it should fix issue #501. I observed the same freezing in github action and this fixes properly handling undefined fixes it.