From fe88b3a4de6fb0ae05fd978d0fa07adba0c8431b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?=
 <1005065+DeepDiver1975@users.noreply.github.com>
Date: Thu, 13 Aug 2026 00:34:55 +0200
Subject: [PATCH] fix(configManager): rethrow errors instead of masking them as
 a TypeError
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`loadYaml` handled failures from `repos.getContent` in a `.catch()` that
logged the error but neither rethrew nor returned a value. The awaited
expression therefore resolved to `undefined`, and the next statement
dereferenced `response.data`, so every config-read failure surfaced as:

    TypeError: Cannot read properties of undefined (reading 'data')

That TypeError carries no `.status`, so the `if (e.status === 404) return
null` in the enclosing catch never matched and the real HTTP error was
lost. A missing settings file aborted the run instead of returning null,
and a 403/500 was indistinguishable from a 404.

`lib/settings.js` already rethrows from the equivalent `.catch()`; this
brings `configManager` in line with it.

Adds test/unit/lib/configManager.test.js, which had no coverage: a 404
now returns null, and a non-404 rejects with the original error object so
its status survives. Three of the seven tests fail without this change.

Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
---
 lib/configManager.js                |   1 +
 test/unit/lib/configManager.test.js | 100 ++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 test/unit/lib/configManager.test.js

diff --git a/lib/configManager.js b/lib/configManager.js
index 7f1bb39b0..e9b172203 100644
--- a/lib/configManager.js
+++ b/lib/configManager.js
@@ -21,6 +21,7 @@ module.exports = class ConfigManager {
       const params = Object.assign(repo, { path: filePath, ref: this.ref })
       const response = await this.context.octokit.rest.repos.getContent(params).catch(e => {
         this.log.error(`Error getting settings ${e}`)
+        throw e
       })
 
       // Ignore in case path is a folder
diff --git a/test/unit/lib/configManager.test.js b/test/unit/lib/configManager.test.js
new file mode 100644
index 000000000..3db68dd7c
--- /dev/null
+++ b/test/unit/lib/configManager.test.js
@@ -0,0 +1,100 @@
+/* eslint-disable no-undef */
+const ConfigManager = require('../../../lib/configManager')
+
+describe('configManager', () => {
+  let context
+
+  beforeEach(() => {
+    context = {
+      repo: () => { return { owner: 'test-org', repo: 'admin' } },
+      octokit: {
+        rest: {
+          repos: {
+            getContent: jest.fn()
+          }
+        }
+      },
+      log: {
+        debug: jest.fn(),
+        info: jest.fn(),
+        error: jest.fn()
+      }
+    }
+  })
+
+  describe('loadYaml', () => {
+    it('returns the parsed YAML content when the file is fetched successfully', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      context.octokit.rest.repos.getContent.mockResolvedValue({
+        data: { content: Buffer.from('key: value').toString('base64') }
+      })
+
+      const result = await configManager.loadYaml('.github/settings.yml')
+
+      expect(result).toEqual({ key: 'value' })
+      expect(context.octokit.rest.repos.getContent).toHaveBeenCalledWith({
+        owner: 'test-org',
+        repo: 'admin',
+        path: '.github/settings.yml',
+        ref: 'main'
+      })
+    })
+
+    it('returns null when the path is a folder', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      context.octokit.rest.repos.getContent.mockResolvedValue({ data: [] })
+
+      await expect(configManager.loadYaml('.github')).resolves.toBeNull()
+    })
+
+    it('returns undefined when the path is a symlink or submodule', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      context.octokit.rest.repos.getContent.mockResolvedValue({ data: { content: null } })
+
+      await expect(configManager.loadYaml('.github/settings.yml')).resolves.toBeUndefined()
+    })
+
+    it('returns null when the file does not exist', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      const notFound = new Error('Not Found')
+      notFound.status = 404
+      context.octokit.rest.repos.getContent.mockRejectedValue(notFound)
+
+      await expect(configManager.loadYaml('.github/settings.yml')).resolves.toBeNull()
+    })
+
+    it('propagates a non-404 error instead of masking it', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      const serverError = new Error('Internal Server Error')
+      serverError.status = 500
+      context.octokit.rest.repos.getContent.mockRejectedValue(serverError)
+
+      await expect(configManager.loadYaml('.github/settings.yml')).rejects.toThrow('Internal Server Error')
+    })
+
+    it('propagates the original error object so its status is preserved', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      const forbidden = new Error('Forbidden')
+      forbidden.status = 403
+      context.octokit.rest.repos.getContent.mockRejectedValue(forbidden)
+
+      await expect(configManager.loadYaml('.github/settings.yml')).rejects.toBe(forbidden)
+    })
+  })
+
+  describe('loadGlobalSettingsYaml', () => {
+    it('loads the settings file from the configured config path', async () => {
+      const configManager = new ConfigManager(context, 'main')
+      context.octokit.rest.repos.getContent.mockResolvedValue({
+        data: { content: Buffer.from('repository:\n  has_wiki: false').toString('base64') }
+      })
+
+      const result = await configManager.loadGlobalSettingsYaml()
+
+      expect(result).toEqual({ repository: { has_wiki: false } })
+      expect(context.octokit.rest.repos.getContent).toHaveBeenCalledWith(
+        expect.objectContaining({ path: '.github/settings.yml' })
+      )
+    })
+  })
+})