Skip to content

Commit 9e47ae6

Browse files
Fix "Cannot set properties of undefined" error in validated() method when credentials are missing (#170)
This PR fixes a critical issue where the plugin would crash with `Cannot set properties of undefined (setting 'installId')` when the homebridge config file didn't have a properly initialized `credentials` object for the August platform. ## Problem The error occurred in the `validated()` method when trying to set properties on `pluginConfig.credentials`: ```typescript pluginConfig.credentials.installId = this.config.credentials?.installId ``` This happened because: 1. When `credentials` was completely missing from the config file, it would be `undefined` 2. When `credentials` was explicitly set to `null`, the existing check `typeof pluginConfig.credentials !== 'object'` would pass (since `typeof null === 'object'` in JavaScript), but then trying to set properties on `null` would fail ## Solution Updated the `pluginConfig()` method to properly handle both `null` and `undefined` credentials: ```typescript // Before: Only checked for non-object types if (typeof pluginConfig.credentials !== 'object') { throw new TypeError('pluginConfig.credentials is not an object') } // After: Explicitly check for null and undefined, then initialize if (typeof pluginConfig.credentials !== 'object' || pluginConfig.credentials === null) { pluginConfig.credentials = {} } ``` Now when the `validated()` method runs, it can safely set properties on `pluginConfig.credentials` because the object is guaranteed to exist. ## Testing Added comprehensive tests to verify: - Undefined credentials in config file are handled gracefully - Null credentials in config file are handled gracefully - The credentials object is properly initialized and the installId is saved All existing tests continue to pass, ensuring no regressions. Fixes #169. <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/homebridge-plugins/homebridge-august/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Donavan Becker <[email protected]> Co-authored-by: donavanbecker <[email protected]>
1 parent c5a9a0b commit 9e47ae6

File tree

2 files changed

+85
-5
lines changed

2 files changed

+85
-5
lines changed

src/platform.test.ts

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,18 @@ import type { AugustPlatformConfig } from './settings.js'
88

99
// Mock august-yale module
1010
vi.mock('august-yale', () => {
11+
const MockAugust = vi.fn().mockImplementation(() => ({
12+
end: vi.fn(),
13+
}))
14+
15+
// Type assertion to add static methods
16+
const MockConstructor = MockAugust as any
17+
MockConstructor.details = vi.fn()
18+
MockConstructor.authorize = vi.fn()
19+
MockConstructor.validate = vi.fn()
20+
1121
return {
12-
default: {
13-
details: vi.fn(),
14-
},
22+
default: MockConstructor,
1523
}
1624
})
1725

@@ -167,4 +175,74 @@ describe('AugustPlatform', () => {
167175
expect(lockSpy).toHaveBeenCalledWith([mockDevice])
168176
})
169177
})
178+
179+
describe('validated method', () => {
180+
it('should handle undefined credentials in config file gracefully', async () => {
181+
// Mock config file with undefined credentials
182+
const mockConfigFile = {
183+
platforms: [
184+
{
185+
platform: 'August',
186+
name: 'Test August',
187+
// credentials is undefined here
188+
},
189+
],
190+
}
191+
192+
vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockConfigFile))
193+
194+
platform = new AugustPlatform(mockLog, mockConfig, mockApi)
195+
196+
// This should not throw an error when trying to set installId
197+
await expect(platform.validated()).resolves.toBeUndefined()
198+
})
199+
200+
it('should handle null credentials in config file gracefully', async () => {
201+
// Mock config file with null credentials
202+
const mockConfigFile = {
203+
platforms: [
204+
{
205+
platform: 'August',
206+
name: 'Test August',
207+
credentials: null,
208+
},
209+
],
210+
}
211+
212+
vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockConfigFile))
213+
214+
platform = new AugustPlatform(mockLog, mockConfig, mockApi)
215+
216+
// This should not throw an error when trying to set installId
217+
await expect(platform.validated()).resolves.toBeUndefined()
218+
})
219+
220+
it('should initialize credentials object and save installId when credentials are missing', async () => {
221+
// Mock config file with undefined credentials
222+
const mockConfigFile = {
223+
platforms: [
224+
{
225+
platform: 'August',
226+
name: 'Test August',
227+
// credentials is undefined here
228+
},
229+
],
230+
}
231+
232+
vi.mocked(readFileSync).mockReturnValue(JSON.stringify(mockConfigFile))
233+
234+
platform = new AugustPlatform(mockLog, mockConfig, mockApi)
235+
236+
await platform.validated()
237+
238+
// Verify that writeFileSync was called (meaning the config was updated)
239+
expect(writeFileSync).toHaveBeenCalled()
240+
241+
// Verify the credentials object was created and populated
242+
const writeCall = vi.mocked(writeFileSync).mock.calls[0]
243+
const savedConfig = JSON.parse(writeCall[1] as string)
244+
expect(savedConfig.platforms[0].credentials).toBeDefined()
245+
expect(savedConfig.platforms[0].credentials.installId).toBeDefined()
246+
})
247+
})
170248
})

src/platform.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,10 @@ export class AugustPlatform implements DynamicPlatformPlugin {
225225
throw new Error(`Cannot find config for ${PLATFORM_NAME} in platforms array`)
226226
}
227227
// check the .credentials is an object before doing object things with it
228-
if (typeof pluginConfig.credentials !== 'object') {
229-
throw new TypeError('pluginConfig.credentials is not an object')
228+
// Note: typeof null === 'object' is true, so we need to explicitly check for null and undefined
229+
if (typeof pluginConfig.credentials !== 'object' || pluginConfig.credentials === null) {
230+
// Initialize credentials as an empty object if it doesn't exist or is null
231+
pluginConfig.credentials = {}
230232
}
231233
return { pluginConfig, currentConfig }
232234
}

0 commit comments

Comments
 (0)