From d772583860493965d932103b8493a69cbe9f34bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franti=C5=A1ek=20P=C3=A1c?= Date: Tue, 10 Jun 2025 18:14:44 +0200 Subject: [PATCH 1/2] fix: handle undefined values when handling results Should fix issue #501 --- lib/configManager.js | 5 +++++ lib/settings.js | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/configManager.js b/lib/configManager.js index 58f5bb436..a5e1d2142 100644 --- a/lib/configManager.js +++ b/lib/configManager.js @@ -21,8 +21,13 @@ module.exports = class ConfigManager { const params = Object.assign(repo, { path: filePath, ref: this.ref }) const response = await this.context.octokit.repos.getContent(params).catch(e => { this.log.error(`Error getting settings ${e}`) + return null }) + if (!response) { + return null + } + // Ignore in case path is a folder // - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-directory if (Array.isArray(response.data)) { diff --git a/lib/settings.js b/lib/settings.js index 20e711672..d405ba9b3 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -203,7 +203,7 @@ 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') } else if (!(res.action?.additions === null && res.action?.deletions === null && res.action?.modifications === null)) { if (!stats.changes[res.plugin]) { stats.changes[res.plugin] = {} @@ -211,7 +211,7 @@ class Settings { 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'}`) } } }) @@ -242,6 +242,11 @@ ${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}` + } + if (y.type === 'ERROR') { error = true return `${x} @@ -249,9 +254,6 @@ ${this.results.reduce((x, y) => { } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) { return `${x}` } else { - if (y.action === undefined) { - return `${x}` - } return `${x} ✋ ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` } From 252fd4b533b2663b0ce90a9e01cd3605d4b0e630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franti=C5=A1ek=20P=C3=A1c?= Date: Thu, 26 Jun 2025 19:27:38 +0200 Subject: [PATCH 2/2] fixup! fix: handle undefined values when handling results --- lib/settings.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index d405ba9b3..9cc309d3e 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -10,10 +10,10 @@ const env = require('./env') const CONFIG_PATH = env.CONFIG_PATH const eta = new Eta({ views: path.join(__dirname) }) const SCOPE = { ORG: 'org', REPO: 'repo' } // Determine if the setting is a org setting or repo setting -const yaml = require('js-yaml'); +const yaml = require('js-yaml') class Settings { - static fileCache = {}; + static fileCache = {} static async syncAll (nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) @@ -170,10 +170,10 @@ class Settings { // remove duplicate rows in this.results this.results = this.results.filter((thing, index, self) => { - return index === self.findIndex((t) => { - return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin - }) + return index === self.findIndex((t) => { + return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin }) + }) let error = false // Different logic @@ -203,7 +203,7 @@ class Settings { if (!stats.errors[res.repo]) { stats.errors[res.repo] = [] } - stats.errors[res.repo].push(res.action ?? 'Unknown error') + stats.errors[res.repo].push(res.action ?? { msg: 'Unknown error' }) } else if (!(res.action?.additions === null && res.action?.deletions === null && res.action?.modifications === null)) { if (!stats.changes[res.plugin]) { stats.changes[res.plugin] = {} @@ -211,7 +211,7 @@ class Settings { if (!stats.changes[res.plugin][res.repo]) { stats.changes[res.plugin][res.repo] = [] } - stats.changes[res.plugin][res.repo].push(`${res.action ?? 'No action details'}`) + stats.changes[res.plugin][res.repo].push(res.action ?? { msg: 'No action details' }) } } }) @@ -244,7 +244,7 @@ ${this.results.reduce((x, y) => { } // Skip results that don't have an action property or have undefined action if (!y.action) { - return `${x}` + return x } if (y.type === 'ERROR') { @@ -252,7 +252,7 @@ ${this.results.reduce((x, y) => { return `${x} ❗ ${y.action.msg} ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) { - return `${x}` + return x } else { return `${x} ✋ ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` @@ -302,7 +302,7 @@ ${this.results.reduce((x, y) => { } } - async updateRepos(repo) { + async updateRepos (repo) { this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs() // Keeping this as is instead of doing an object assign as that would cause `Cannot read properties of undefined (reading 'startsWith')` error // Copilot code review would recoommend using object assign but that would cause the error @@ -370,7 +370,6 @@ ${this.results.reduce((x, y) => { } } - async updateAll () { // this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs(this.github, this.repo, this.log) // this.repoConfigs = this.repoConfigs || await this.getRepoConfigs(this.github, this.repo, this.log) @@ -793,14 +792,14 @@ ${this.results.reduce((x, y) => { * @param params Params to fetch the file with * @return The parsed YAML file */ - async loadYaml(filePath) { + async loadYaml (filePath) { try { const repo = { owner: this.repo.owner, repo: env.ADMIN_REPO } const params = Object.assign(repo, { path: filePath, ref: this.ref }) - const namespacedFilepath = `${this.repo.owner}/${filePath}`; + const namespacedFilepath = `${this.repo.owner}/${filePath}` // If the filepath already exists in the fileCache, add the etag to the params // to check if the file has changed @@ -900,7 +899,6 @@ ${this.results.reduce((x, y) => { } } - async getSubOrgRepositories (subOrgProperties) { const organizationName = this.repo.owner try {