From c844711f6dbeb91c4f6d6ef3380ff485d1fac801 Mon Sep 17 00:00:00 2001 From: bymyself Date: Fri, 19 Sep 2025 01:38:37 -0700 Subject: [PATCH] [fix] Resolve Sentry issue CLOUD-FRONTEND-STAGING-13 - TypeError on undefined device Fixes TypeError: Cannot read properties of undefined (reading 'name') occurring in DeviceInfo component when device prop is undefined. Changes: - DeviceInfo: Add null checks and graceful error handling for undefined device prop - DeviceInfo: Enhanced formatValue to return 'N/A' for null/undefined values - SystemStatsPanel: Add guard clauses for empty devices array - SystemStatsPanel: Display helpful message when no devices available - Added monitoring to log when devices array is missing for debugging - Added i18n keys for error messages Root cause: Component attempted to access properties on undefined device object when SystemStats API returned empty devices array or malformed data. Solution: Defensive programming with null checks and user-friendly error messages. Tests updated to verify graceful handling instead of throwing errors. Sentry URL: https://comfy-org.sentry.io/issues/6804418395/?project=4509681221369857 --- src/components/common/DeviceInfo.vue | 11 ++- src/components/common/SystemStatsPanel.vue | 40 ++++++--- src/locales/en/main.json | 2 + .../components/common/DeviceInfo.test.ts | 87 ++++++++++--------- 4 files changed, 85 insertions(+), 55 deletions(-) diff --git a/src/components/common/DeviceInfo.vue b/src/components/common/DeviceInfo.vue index 8e00fd16b..5cc9378d0 100644 --- a/src/components/common/DeviceInfo.vue +++ b/src/components/common/DeviceInfo.vue @@ -1,5 +1,5 @@ diff --git a/src/locales/en/main.json b/src/locales/en/main.json index 3abd85335..5a9b21483 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -29,6 +29,8 @@ "insert": "Insert", "systemInfo": "System Info", "devices": "Devices", + "deviceNotAvailable": "Device information is not available.", + "noDevicesDetected": "No devices detected. This may occur if no GPU devices are available or device enumeration failed.", "about": "About", "add": "Add", "confirm": "Confirm", diff --git a/tests-ui/tests/components/common/DeviceInfo.test.ts b/tests-ui/tests/components/common/DeviceInfo.test.ts index 37fad2be7..48cb6ef1e 100644 --- a/tests-ui/tests/components/common/DeviceInfo.test.ts +++ b/tests-ui/tests/components/common/DeviceInfo.test.ts @@ -27,8 +27,8 @@ describe('DeviceInfo', () => { expect(wrapper.text()).toContain('NVIDIA GeForce RTX 4090') expect(wrapper.text()).toContain('cuda') - expect(wrapper.text()).toContain('22.9 GB') // vram_total formatted - expect(wrapper.text()).toContain('19.1 GB') // vram_free formatted + expect(wrapper.text()).toContain('22.88 GB') // vram_total formatted + expect(wrapper.text()).toContain('19.15 GB') // vram_free formatted }) it('should display all device columns', () => { @@ -45,23 +45,23 @@ describe('DeviceInfo', () => { }) }) - describe('Sentry Issue CLOUD-FRONTEND-STAGING-13: undefined device prop', () => { - it('should throw TypeError when device prop is undefined', () => { - // This test reproduces the exact Sentry error - expect(() => { - createWrapper(undefined as any) - }).toThrow() + describe('Sentry Issue CLOUD-FRONTEND-STAGING-13: undefined device prop - FIXED', () => { + it('should gracefully handle undefined device prop instead of throwing', () => { + // Previously this threw TypeError, now it should render error message + const wrapper = createWrapper(undefined as any) + expect(wrapper.text()).toContain('g.deviceNotAvailable') + expect(wrapper.find('.text-red-500').exists()).toBe(true) }) - it('should throw TypeError when accessing undefined device properties', () => { - // Test the specific error: Cannot read properties of undefined (reading 'name') - expect(() => { - const wrapper = mount(DeviceInfo, { - props: { device: undefined as any } - }) - // This will trigger the error when Vue tries to render the template - wrapper.html() - }).toThrow(TypeError) + it('should gracefully handle undefined device properties instead of throwing', () => { + // Previously threw TypeError: Cannot read properties of undefined (reading 'name') + // Now should render fallback message + const wrapper = mount(DeviceInfo, { + props: { device: undefined as any } + }) + + expect(wrapper.html()).toContain('g.deviceNotAvailable') + expect(() => wrapper.html()).not.toThrow() }) it('should fail when formatValue tries to access undefined device fields', () => { @@ -84,19 +84,20 @@ describe('DeviceInfo', () => { }) }) - describe('Edge cases that could lead to undefined device', () => { - it('should handle device with missing required fields', () => { + describe('Edge cases that could lead to undefined device - FIXED', () => { + it('should gracefully handle device with missing required fields', () => { const incompleteDevice = { name: 'Test Device' // Missing required fields: type, index, vram_total, etc. } as any - expect(() => { - createWrapper(incompleteDevice) - }).toThrow() + const wrapper = createWrapper(incompleteDevice) + expect(wrapper.exists()).toBe(true) + expect(wrapper.text()).toContain('Test Device') + expect(wrapper.text()).toContain('N/A') // Missing fields show as N/A }) - it('should handle device with null values', () => { + it('should handle device with null values gracefully', () => { const deviceWithNulls = { name: null, type: null, @@ -108,24 +109,25 @@ describe('DeviceInfo', () => { } as any const wrapper = createWrapper(deviceWithNulls) - // The component should render but may show null values expect(wrapper.exists()).toBe(true) + expect(wrapper.text()).toContain('N/A') // Null values show as N/A }) }) - describe('SystemStatsPanel integration scenarios', () => { - it('should fail when devices array is empty and accessing devices[0]', () => { + describe('SystemStatsPanel integration scenarios - FIXED', () => { + it('should gracefully handle when devices array is empty and accessing devices[0]', () => { // This simulates the scenario where props.stats.devices[0] is undefined // because the devices array is empty const emptyDevicesArray: DeviceStats[] = [] - expect(() => { - const deviceFromEmptyArray = emptyDevicesArray[0] // undefined - createWrapper(deviceFromEmptyArray) - }).toThrow() + const deviceFromEmptyArray = emptyDevicesArray[0] // undefined + const wrapper = createWrapper(deviceFromEmptyArray) + + expect(wrapper.text()).toContain('g.deviceNotAvailable') + expect(() => createWrapper(deviceFromEmptyArray)).not.toThrow() }) - it('should fail when SystemStats API returns malformed data', () => { + it('should gracefully handle when SystemStats API returns malformed data', () => { // Simulate API returning data that doesn't match expected schema const malformedApiResponse = { system: { @@ -134,15 +136,16 @@ describe('DeviceInfo', () => { devices: null // This should be an array but API returned null } - expect(() => { - const deviceFromMalformedData = malformedApiResponse.devices?.[0] - createWrapper(deviceFromMalformedData) - }).toThrow() + const deviceFromMalformedData = malformedApiResponse.devices?.[0] + const wrapper = createWrapper(deviceFromMalformedData) + + expect(wrapper.text()).toContain('g.deviceNotAvailable') + expect(() => createWrapper(deviceFromMalformedData)).not.toThrow() }) }) - describe('formatValue function edge cases', () => { - it('should handle undefined values in VRAM fields', () => { + describe('formatValue function edge cases - FIXED', () => { + it('should gracefully handle undefined values in VRAM fields', () => { const deviceWithUndefinedVram = { name: 'Test Device', type: 'cuda', @@ -153,10 +156,12 @@ describe('DeviceInfo', () => { torch_vram_free: undefined } as any - // The component should render but formatValue might fail - expect(() => { - createWrapper(deviceWithUndefinedVram) - }).toThrow() + // Previously would fail, now should show N/A for undefined VRAM values + const wrapper = createWrapper(deviceWithUndefinedVram) + expect(wrapper.exists()).toBe(true) + expect(wrapper.text()).toContain('Test Device') + expect(wrapper.text()).toContain('cuda') + expect(wrapper.text()).toContain('N/A') // Undefined VRAM values show as N/A }) }) })