From 9cbfb5ae758de76940b133dc860e09e390c19690 Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Tue, 5 May 2026 18:31:27 -0600 Subject: [PATCH 1/4] test: add failing tests for validation error cases in reducer Add comprehensive test coverage for error handling in node.reducer for: - NoValueCollector update rejection - Undefined value detection - Type validation for SingleValueCollector (must be string) - MultiValueCollector object rejection - PollingCollector update rejection All tests fail as expected in RED phase (throws are unhandled by reducer). --- .../src/lib/node.reducer.test.ts | 120 +++++++++++++++++- 1 file changed, 117 insertions(+), 3 deletions(-) diff --git a/packages/davinci-client/src/lib/node.reducer.test.ts b/packages/davinci-client/src/lib/node.reducer.test.ts index d1fcc95655..65a9b00f57 100644 --- a/packages/davinci-client/src/lib/node.reducer.test.ts +++ b/packages/davinci-client/src/lib/node.reducer.test.ts @@ -381,7 +381,7 @@ describe('The node collector reducer', () => { expect(() => nodeCollectorReducer(state, action)).toThrowError('No collector found to update'); }); - it('should throw with no Action Collector', () => { + it('should set error on ActionCollector when update is attempted', () => { const action = { type: 'node/update', payload: { @@ -421,8 +421,122 @@ describe('The node collector reducer', () => { }, }, ]; - expect(() => nodeCollectorReducer(state, action)).toThrowError( - 'ActionCollectors are read-only', + const result = nodeCollectorReducer(state, action); + const collector = result.find((c) => c.id === 'submit-1'); + expect(collector?.error).toBe('ActionCollectors are read-only'); + }); + + it('should set error on NoValueCollector when update is attempted', () => { + const state: QrCodeCollector[] = [ + { + category: 'NoValueCollector', + error: null, + type: 'QrCodeCollector', + id: 'qr-0', + name: 'qr', + output: { + key: 'qr', + label: 'QR Code', + type: 'QR_CODE', + src: 'data:image/png;base64,abc', + }, + }, + ]; + const action = { type: 'node/update', payload: { id: 'qr-0', value: 'anything' } }; + const result = nodeCollectorReducer(state, action); + expect(result.find((c) => c.id === 'qr-0')?.error).toBe( + 'NoValueCollectors, like ReadOnlyCollectors, are read-only', + ); + }); + + it('should set error on collector when value is undefined', () => { + const state: TextCollector[] = [ + { + category: 'SingleValueCollector', + error: null, + type: 'TextCollector', + id: 'username-0', + name: 'username', + input: { key: 'username', value: '', type: 'TEXT' }, + output: { key: 'username', label: 'Username', type: 'TEXT', value: '' }, + }, + ]; + const action = { + type: 'node/update', + payload: { id: 'username-0', value: undefined as unknown as string }, + }; + const result = nodeCollectorReducer(state, action); + expect(result.find((c) => c.id === 'username-0')?.error).toBe( + 'Value argument cannot be undefined', + ); + }); + + it('should set error on SingleValueCollector when value is not a string', () => { + const state: TextCollector[] = [ + { + category: 'SingleValueCollector', + error: null, + type: 'TextCollector', + id: 'username-0', + name: 'username', + input: { key: 'username', value: '', type: 'TEXT' }, + output: { key: 'username', label: 'Username', type: 'TEXT', value: '' }, + }, + ]; + const action = { + type: 'node/update', + payload: { id: 'username-0', value: 42 as unknown as string }, + }; + const result = nodeCollectorReducer(state, action); + expect(result.find((c) => c.id === 'username-0')?.error).toBe( + 'Value argument must be a string', + ); + }); + + it('should set error on MultiValueCollector when value is an object', () => { + const state: MultiSelectCollector[] = [ + { + category: 'MultiValueCollector', + error: null, + type: 'MultiSelectCollector', + id: 'multi-0', + name: 'multi', + input: { key: 'multi', value: [], type: 'MULTI_SELECT', validation: null }, + output: { + key: 'multi', + label: 'Multi', + type: 'MULTI_SELECT', + value: [], + options: [{ label: 'A', value: 'a' }], + }, + }, + ]; + const action = { + type: 'node/update', + payload: { id: 'multi-0', value: { bad: true } as unknown as string }, + }; + const result = nodeCollectorReducer(state, action); + expect(result.find((c) => c.id === 'multi-0')?.error).toBe( + 'MultiValueCollector does not accept an object', + ); + }); + + it('should set error on PollingCollector when update is attempted', () => { + const state: PollingCollector[] = [ + { + category: 'SingleValueAutoCollector', + error: null, + type: 'PollingCollector', + id: 'poll-0', + name: 'poll', + input: { key: 'poll', value: '', type: 'POLLING' }, + output: { key: 'poll', type: 'POLLING', config: { pollInterval: 1000, pollRetries: 3 } }, + }, + ]; + const action = { type: 'node/update', payload: { id: 'poll-0', value: 'anything' } }; + const result = nodeCollectorReducer(state, action); + expect(result.find((c) => c.id === 'poll-0')?.error).toBe( + 'This collector type does not support value updates', ); }); From 90d4f9ecfcbaf9af2711fcf88710627febd82da1 Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Tue, 5 May 2026 18:38:54 -0600 Subject: [PATCH 2/4] fix(davinci-client): replace reducer throws with collector.error state writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates thrown errors from updateCollectorValues — all validation failures now write to collector.error instead, keeping Redux state predictable and observable rather than blowing up the call stack. Also adds an explicit PollingCollector type guard before the category branches so that polling collectors (which are categorized as SingleValueAutoCollector) are correctly rejected as read-only. --- .../davinci-client/src/lib/node.reducer.ts | 120 ++++++++++++------ 1 file changed, 81 insertions(+), 39 deletions(-) diff --git a/packages/davinci-client/src/lib/node.reducer.ts b/packages/davinci-client/src/lib/node.reducer.ts index 1dd74a6d53..d51dfd0732 100644 --- a/packages/davinci-client/src/lib/node.reducer.ts +++ b/packages/davinci-client/src/lib/node.reducer.ts @@ -188,17 +188,41 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build */ .addCase(updateCollectorValues, (state, action) => { const collector = state.find((collector) => collector.id === action.payload.id); + if (!collector) { - throw new Error('No collector found to update'); + state.push({ + category: 'UnknownCollector', + error: 'No collector found to update', + type: 'UnknownCollector', + id: action.payload.id, + name: action.payload.id, + output: { + key: action.payload.id, + label: action.payload.id, + type: 'UnknownCollector', + }, + }); + return; } + if (collector.category === 'ActionCollector') { - throw new Error('ActionCollectors are read-only'); + collector.error = 'ActionCollectors are read-only'; + return; } + if (collector.category === 'NoValueCollector') { - throw new Error('NoValueCollectors, like ReadOnlyCollectors, are read-only'); + collector.error = 'NoValueCollectors, like ReadOnlyCollectors, are read-only'; + return; } + + if (collector.type === 'PollingCollector') { + collector.error = 'This collector type does not support value updates'; + return; + } + if (action.payload.value === undefined) { - throw new Error('Value argument cannot be undefined'); + collector.error = 'Value argument cannot be undefined'; + return; } if ( @@ -215,7 +239,8 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build } if (typeof action.payload.value !== 'string') { - throw new Error('Value argument must be a string'); + collector.error = 'Value argument must be a string'; + return; } collector.input.value = action.payload.value; return; @@ -223,7 +248,8 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build if (collector.category === 'MultiValueCollector') { if (typeof action.payload.value !== 'string' && !Array.isArray(action.payload.value)) { - throw new Error('MultiValueCollector does not accept an object'); + collector.error = 'MultiValueCollector does not accept an object'; + return; } if (Array.isArray(action.payload.value)) { collector.input.value = [...action.payload.value]; @@ -234,99 +260,115 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build } if (collector.type === 'DeviceAuthenticationCollector') { - const inputValue = action.payload.value; - if (typeof inputValue !== 'string') { - throw new Error('Value argument must be a string'); + if (typeof action.payload.value !== 'string') { + collector.error = 'Value argument must be a string'; + return; } - - // Iterate through the options object and find option to update - const option = collector.output.options.find((option) => option.value === inputValue); - + const option = collector.output.options.find( + (option) => option.value === action.payload.value, + ); if (!option) { - throw new Error('No option found matching value to update'); + collector.error = 'No option found matching value to update'; + return; } - - // Remap values back to DaVinci spec collector.input.value = { type: option.type, id: option.value, value: option.content, }; + return; } if (collector.type === 'DeviceRegistrationCollector') { - const inputValue = action.payload.value; - if (typeof inputValue !== 'string') { - throw new Error('Value argument must be a string'); + if (typeof action.payload.value !== 'string') { + collector.error = 'Value argument must be a string'; + return; } - - // Iterate through the options object and find option to update - const option = collector.output.options.find((option) => option.value === inputValue); - + const option = collector.output.options.find( + (option) => option.value === action.payload.value, + ); if (!option) { - throw new Error('No option found matching value to update'); + collector.error = 'No option found matching value to update'; + return; } - collector.input.value = option.type; + return; } if (collector.type === 'PhoneNumberCollector') { if (typeof action.payload.id !== 'string') { - throw new Error('Index argument must be a string'); + collector.error = 'Index argument must be a string'; + return; } if (typeof action.payload.value !== 'object') { - throw new Error('Value argument must be an object'); + collector.error = 'Value argument must be an object'; + return; } if (!('phoneNumber' in action.payload.value) || !('countryCode' in action.payload.value)) { - throw new Error('Value argument must contain a phoneNumber and countryCode property'); + collector.error = 'Value argument must contain a phoneNumber and countryCode property'; + return; } collector.input.value = action.payload.value; + return; } if (collector.type === 'PhoneNumberExtensionCollector') { if (typeof action.payload.id !== 'string') { - throw new Error('Index argument must be a string'); + collector.error = 'Index argument must be a string'; + return; } if (typeof action.payload.value !== 'object') { - throw new Error('Value argument must be an object'); + collector.error = 'Value argument must be an object'; + return; } if ( !('phoneNumber' in action.payload.value) || !('countryCode' in action.payload.value) || !('extension' in action.payload.value) ) { - throw new Error( - 'Value argument must contain a phoneNumber, countryCode, and extension property', - ); + collector.error = + 'Value argument must contain a phoneNumber, countryCode, and extension property'; + return; } collector.input.value = action.payload.value; + return; } if (collector.type === 'FidoRegistrationCollector') { if (typeof action.payload.id !== 'string') { - throw new Error('Index argument must be a string'); + collector.error = 'Index argument must be a string'; + return; } if (typeof action.payload.value !== 'object') { - throw new Error('Value argument must be an object'); + collector.error = 'Value argument must be an object'; + return; } if (!('attestationValue' in action.payload.value)) { - throw new Error('Value argument must contain an attestationValue property'); + collector.error = 'Value argument must contain an attestationValue property'; + return; } collector.input.value = action.payload.value; + return; } if (collector.type === 'FidoAuthenticationCollector') { if (typeof action.payload.id !== 'string') { - throw new Error('Index argument must be a string'); + collector.error = 'Index argument must be a string'; + return; } if (typeof action.payload.value !== 'object') { - throw new Error('Value argument must be an object'); + collector.error = 'Value argument must be an object'; + return; } if (!('assertionValue' in action.payload.value)) { - throw new Error('Value argument must contain an assertionValue property'); + collector.error = 'Value argument must contain an assertionValue property'; + return; } collector.input.value = action.payload.value; + return; } + + collector.error = 'This collector type does not support value updates'; }) /** * Using the `pollCollectorValues` const (e.g. `'node/poll'`) to add the case From e5a9a2ff6a2d0fa68197080239d6676e1f7cef6d Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Tue, 5 May 2026 18:45:18 -0600 Subject: [PATCH 3/4] fix(davinci-client): clear stale collector error on successful value update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stale validation errors persisted in state after a user corrected their input — a subsequent valid dispatch left the previous error message visible. Mirror the pattern already used in pollCollectorValues by resetting collector.error to null on every successful value-assignment path in updateCollectorValues. --- .../src/lib/node.reducer.test.ts | 19 +++++++++++++++++++ .../davinci-client/src/lib/node.reducer.ts | 8 ++++++++ 2 files changed, 27 insertions(+) diff --git a/packages/davinci-client/src/lib/node.reducer.test.ts b/packages/davinci-client/src/lib/node.reducer.test.ts index 65a9b00f57..97c2c05cb7 100644 --- a/packages/davinci-client/src/lib/node.reducer.test.ts +++ b/packages/davinci-client/src/lib/node.reducer.test.ts @@ -471,6 +471,25 @@ describe('The node collector reducer', () => { ); }); + it('should clear error on collector when a valid value is provided after an error', () => { + const state: TextCollector[] = [ + { + category: 'SingleValueCollector', + error: 'Value argument must be a string', + type: 'TextCollector', + id: 'username-0', + name: 'username', + input: { key: 'username', value: '', type: 'TEXT' }, + output: { key: 'username', label: 'Username', type: 'TEXT', value: '' }, + }, + ]; + const action = { type: 'node/update', payload: { id: 'username-0', value: 'validString' } }; + const result = nodeCollectorReducer(state, action); + const collector = result.find((c) => c.id === 'username-0') as TextCollector | undefined; + expect(collector?.error).toBeNull(); + expect(collector?.input.value).toBe('validString'); + }); + it('should set error on SingleValueCollector when value is not a string', () => { const state: TextCollector[] = [ { diff --git a/packages/davinci-client/src/lib/node.reducer.ts b/packages/davinci-client/src/lib/node.reducer.ts index d51dfd0732..debccdbdce 100644 --- a/packages/davinci-client/src/lib/node.reducer.ts +++ b/packages/davinci-client/src/lib/node.reducer.ts @@ -242,6 +242,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'Value argument must be a string'; return; } + collector.error = null; collector.input.value = action.payload.value; return; } @@ -251,6 +252,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'MultiValueCollector does not accept an object'; return; } + collector.error = null; if (Array.isArray(action.payload.value)) { collector.input.value = [...action.payload.value]; } else { @@ -271,6 +273,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'No option found matching value to update'; return; } + collector.error = null; collector.input.value = { type: option.type, id: option.value, @@ -291,6 +294,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'No option found matching value to update'; return; } + collector.error = null; collector.input.value = option.type; return; } @@ -308,6 +312,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'Value argument must contain a phoneNumber and countryCode property'; return; } + collector.error = null; collector.input.value = action.payload.value; return; } @@ -330,6 +335,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build 'Value argument must contain a phoneNumber, countryCode, and extension property'; return; } + collector.error = null; collector.input.value = action.payload.value; return; } @@ -347,6 +353,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'Value argument must contain an attestationValue property'; return; } + collector.error = null; collector.input.value = action.payload.value; return; } @@ -364,6 +371,7 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build collector.error = 'Value argument must contain an assertionValue property'; return; } + collector.error = null; collector.input.value = action.payload.value; return; } From 4ae3a066448c05150f1d2535bff30383e36bb55b Mon Sep 17 00:00:00 2001 From: Ryan Bas Date: Tue, 5 May 2026 18:47:45 -0600 Subject: [PATCH 4/4] =?UTF-8?q?fix(davinci-client):=20remove=20try/catch?= =?UTF-8?q?=20from=20update=20dispatch=20=E2=80=94=20errors=20are=20now=20?= =?UTF-8?q?state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/davinci-client/src/lib/client.store.ts | 12 ++---------- packages/davinci-client/src/lib/node.reducer.test.ts | 7 +++++-- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/davinci-client/src/lib/client.store.ts b/packages/davinci-client/src/lib/client.store.ts index dc78d00ae0..176ec3cf7f 100644 --- a/packages/davinci-client/src/lib/client.store.ts +++ b/packages/davinci-client/src/lib/client.store.ts @@ -333,16 +333,8 @@ export async function davinci({ } return function (value: CollectorValueTypes, index?: number) { - try { - store.dispatch(nodeSlice.actions.update({ id, value, index })); - return null; - } catch (err) { - const errorMessage = err instanceof Error ? err.message : String(err); - return { - type: 'internal_error', - error: { message: errorMessage, type: 'internal_error' }, - }; - } + store.dispatch(nodeSlice.actions.update({ id, value, index })); + return null; }; }, diff --git a/packages/davinci-client/src/lib/node.reducer.test.ts b/packages/davinci-client/src/lib/node.reducer.test.ts index 97c2c05cb7..0833da1372 100644 --- a/packages/davinci-client/src/lib/node.reducer.test.ts +++ b/packages/davinci-client/src/lib/node.reducer.test.ts @@ -350,7 +350,7 @@ describe('The node collector reducer', () => { ]); }); - it('should throw with no collectors', () => { + it('should add an UnknownCollector with error when no matching collector is found', () => { const action = { type: 'node/update', payload: { @@ -378,7 +378,10 @@ describe('The node collector reducer', () => { }, }, ]; - expect(() => nodeCollectorReducer(state, action)).toThrowError('No collector found to update'); + const result = nodeCollectorReducer(state, action); + const errorCollector = result.find((c) => c.id === 'submit-1'); + expect(errorCollector?.error).toBe('No collector found to update'); + expect(errorCollector?.category).toBe('UnknownCollector'); }); it('should set error on ActionCollector when update is attempted', () => {