From 3b3e2879be467dc6b834cb1b504f5832c65a677a 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 4c566055fe..6b901ef6ad 100644 --- a/packages/davinci-client/src/lib/node.reducer.test.ts +++ b/packages/davinci-client/src/lib/node.reducer.test.ts @@ -385,7 +385,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: { @@ -425,8 +425,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 456b5a4e6a17bb4dcee197463f6d1f6f635cb207 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 | 123 ++++++++++++------ 1 file changed, 83 insertions(+), 40 deletions(-) diff --git a/packages/davinci-client/src/lib/node.reducer.ts b/packages/davinci-client/src/lib/node.reducer.ts index a1ad00c1f7..bd894ed9c3 100644 --- a/packages/davinci-client/src/lib/node.reducer.ts +++ b/packages/davinci-client/src/lib/node.reducer.ts @@ -201,18 +201,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 ( @@ -232,7 +255,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; @@ -240,7 +264,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]; @@ -251,102 +276,116 @@ 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; } const isFidoError = 'type' in action.payload.value && action.payload.value.type === 'fido_error'; if (!isFidoError && !('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 as FidoRegistrationInputValue | GenericError; + 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; } const isFidoError = 'type' in action.payload.value && action.payload.value.type === 'fido_error'; if (!isFidoError && !('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 as FidoAuthenticationInputValue | GenericError; + return; } if (collector.type === 'MetadataCollector') { @@ -355,10 +394,14 @@ export const nodeCollectorReducer = createReducer(initialCollectorValues, (build action.payload.value === null || Array.isArray(action.payload.value) ) { - throw new Error('Value argument must be an object'); + collector.error = 'Value argument must be an object'; + return; } collector.input.value = action.payload.value as Record; + return; } + + collector.error = 'This collector type does not support value updates'; }) /** * Using the `pollCollectorValues` const (e.g. `'node/poll'`) to add the case From d0d8fb57286e7e817eb06a5134ff777692e60f8c 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 6b901ef6ad..f9784e41ac 100644 --- a/packages/davinci-client/src/lib/node.reducer.test.ts +++ b/packages/davinci-client/src/lib/node.reducer.test.ts @@ -475,6 +475,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 bd894ed9c3..dc94b06234 100644 --- a/packages/davinci-client/src/lib/node.reducer.ts +++ b/packages/davinci-client/src/lib/node.reducer.ts @@ -258,6 +258,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; } @@ -267,6 +268,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 { @@ -287,6 +289,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, @@ -307,6 +310,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; } @@ -324,6 +328,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; } @@ -346,6 +351,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; } @@ -365,6 +371,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 as FidoRegistrationInputValue | GenericError; return; } @@ -384,6 +391,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 as FidoAuthenticationInputValue | GenericError; return; } From f25719ba2aefbb93be5b7e203e0d2e6af750a0bd 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 a95401dd91..617928b12a 100644 --- a/packages/davinci-client/src/lib/client.store.ts +++ b/packages/davinci-client/src/lib/client.store.ts @@ -336,16 +336,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 f9784e41ac..58dd8cbdb3 100644 --- a/packages/davinci-client/src/lib/node.reducer.test.ts +++ b/packages/davinci-client/src/lib/node.reducer.test.ts @@ -354,7 +354,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: { @@ -382,7 +382,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', () => {