From bd780366e86618ef0d1f278e19cde4e1b43e61ad Mon Sep 17 00:00:00 2001 From: csorrentino Date: Thu, 23 Oct 2025 23:14:44 +0200 Subject: [PATCH] refactor(forms): add onError callback to validateHttp for HTTP errors Adds onError callback inside validateHttp validator in signal forms PR-Close: #63949 (cherry picked from commit a5678f6f2b58470b86507cb78c2bd19bf19169a9) --- goldens/public-api/forms/signals/index.api.md | 6 ++- packages/forms/signals/src/api/async.ts | 25 ++++++++--- .../src/api/validators/standard_schema.ts | 3 +- .../forms/signals/test/node/resource.spec.ts | 44 +++++++++++++++++-- .../forms/signals/test/node/submit.spec.ts | 3 +- .../test/node/validation_status.spec.ts | 39 ++++++++++------ 6 files changed, 91 insertions(+), 29 deletions(-) diff --git a/goldens/public-api/forms/signals/index.api.md b/goldens/public-api/forms/signals/index.api.md index 495563e3f1d..62dc352d4ad 100644 --- a/goldens/public-api/forms/signals/index.api.md +++ b/goldens/public-api/forms/signals/index.api.md @@ -61,8 +61,9 @@ export type AsyncValidationResult = // @public export interface AsyncValidatorOptions { - readonly errors: MapToErrorsFn; readonly factory: (params: Signal) => ResourceRef; + readonly onError: (error: unknown, ctx: FieldContext) => TreeValidationResult; + readonly onSuccess: MapToErrorsFn; readonly params: (ctx: FieldContext) => TParams; } @@ -233,7 +234,8 @@ export function hidden(path: // @public export interface HttpValidatorOptions { - readonly errors: MapToErrorsFn; + readonly onError: (error: unknown, ctx: FieldContext) => TreeValidationResult; + readonly onSuccess: MapToErrorsFn; readonly options?: HttpResourceOptions; readonly request: ((ctx: FieldContext) => string | undefined) | ((ctx: FieldContext) => HttpResourceRequest | undefined); } diff --git a/packages/forms/signals/src/api/async.ts b/packages/forms/signals/src/api/async.ts index d255eececd9..ceec20c10b0 100644 --- a/packages/forms/signals/src/api/async.ts +++ b/packages/forms/signals/src/api/async.ts @@ -71,7 +71,11 @@ export interface AsyncValidatorOptions< * @returns A reference to the constructed resource. */ readonly factory: (params: Signal) => ResourceRef; - + /** + * A function to handle errors thrown by httpResource (HTTP errors, network errors, etc.). + * Receives the error and the field context, returns a list of validation errors. + */ + readonly onError: (error: unknown, ctx: FieldContext) => TreeValidationResult; /** * A function that takes the resource result, and the current field context and maps it to a list * of validation errors. @@ -83,7 +87,7 @@ export interface AsyncValidatorOptions< * A targeted error will show up as an error on its target field rather than the field being validated. * If a field is not given, the error is assumed to apply to the field being validated. */ - readonly errors: MapToErrorsFn; + readonly onSuccess: MapToErrorsFn; } /** @@ -120,8 +124,13 @@ export interface HttpValidatorOptions; + readonly onSuccess: MapToErrorsFn; + /** + * A function to handle errors thrown by httpResource (HTTP errors, network errors, etc.). + * Receives the error and the field context, returns a list of validation errors. + */ + readonly onError: (error: unknown, ctx: FieldContext) => TreeValidationResult; /** * The options to use when creating the httpResource. */ @@ -163,6 +172,7 @@ export function validateAsync { const res = ctx.state.metadata(RESOURCE)!; + let errors; switch (res.status()) { case 'idle': return undefined; @@ -174,11 +184,11 @@ export function validateAsync); + errors = opts.onSuccess(res.value()!, ctx as FieldContext); return addDefaultField(errors, ctx.field); case 'error': - // TODO: Design error handling for async validation. For now, just throw the error. - throw res.error(); + errors = opts.onError(res.error(), ctx as FieldContext); + return addDefaultField(errors, ctx.field); } }); } @@ -203,6 +213,7 @@ export function validateHttp) => httpResource(request, opts.options), - errors: opts.errors, + onSuccess: opts.onSuccess, + onError: opts.onError, }); } diff --git a/packages/forms/signals/src/api/validators/standard_schema.ts b/packages/forms/signals/src/api/validators/standard_schema.ts index 719f3a1f8e5..1d16779322d 100644 --- a/packages/forms/signals/src/api/validators/standard_schema.ts +++ b/packages/forms/signals/src/api/validators/standard_schema.ts @@ -87,9 +87,10 @@ export function validateStandardSchema (await params)?.issues ?? [], }); }, - errors: (issues, {fieldOf}) => { + onSuccess: (issues, {fieldOf}) => { return issues.map((issue) => standardIssueToFormTreeError(fieldOf(path), issue)); }, + onError: () => {}, }); } diff --git a/packages/forms/signals/test/node/resource.spec.ts b/packages/forms/signals/test/node/resource.spec.ts index 810526fc278..0bfd6fc9238 100644 --- a/packages/forms/signals/test/node/resource.spec.ts +++ b/packages/forms/signals/test/node/resource.spec.ts @@ -161,7 +161,7 @@ describe('resources', () => { return params as Cat[]; }, }), - errors: (cats, {fieldOf}) => { + onSuccess: (cats, {fieldOf}) => { return cats.map((cat, index) => customError({ kind: 'meows_too_much', @@ -170,6 +170,7 @@ describe('resources', () => { }), ); }, + onError: () => null, }); }; @@ -196,13 +197,14 @@ describe('resources', () => { return params as Cat[]; }, }), - errors: (cats, {fieldOf}) => { + onSuccess: (cats, {fieldOf}) => { return customError({ kind: 'meows_too_much', name: cats[0].name, field: fieldOf(p)[0], }); }, + onError: () => null, }); }; @@ -222,8 +224,9 @@ describe('resources', () => { (p) => { validateHttp(p, { request: ({value}) => `/api/check?username=${value()}`, - errors: (available: boolean) => + onSuccess: (available: boolean) => available ? undefined : customError({kind: 'username-taken'}), + onError: () => null, }); }, {injector}, @@ -266,8 +269,9 @@ describe('resources', () => { required(address.street); validateHttp(address, { request: ({value}) => ({url: '/checkaddress', params: {...value()}}), - errors: (message: string, {fieldOf}) => + onSuccess: (message: string, {fieldOf}) => customError({message, field: fieldOf(address.street)}), + onError: () => null, }); }); const addressForm = form(addressModel, addressSchema, {injector}); @@ -286,4 +290,36 @@ describe('resources', () => { customError({message: 'Invalid!', field: addressForm.street}), ]); }); + it('should call onError handler when http validation fails', async () => { + const addressModel = signal
({street: '123 Main St', city: '', zip: ''}); + const addressSchema = schema
((address) => { + required(address.street); + validateHttp(address, { + request: ({value}) => ({url: '/checkaddress', params: {...value()}}), + onSuccess: () => undefined, + onError: () => [ + customError({kind: 'address-api-failed', message: 'API is down', field: addressForm}), + ], + }); + }); + + const addressForm = form(addressModel, addressSchema, {injector}); + + TestBed.tick(); + + const req = backend.expectOne('/checkaddress?street=123%20Main%20St&city=&zip='); + req.flush(null, {status: 500, statusText: 'Server Error'}); + + await appRef.whenStable(); + + expect(addressForm().pending()).toBe(false); + expect(addressForm().invalid()).toBe(true); + expect(addressForm().errors()).toEqual([ + customError({ + kind: 'address-api-failed', + message: 'API is down', + field: addressForm, + }), + ]); + }); }); diff --git a/packages/forms/signals/test/node/submit.spec.ts b/packages/forms/signals/test/node/submit.spec.ts index ba06ec662ac..c30867bdb2d 100644 --- a/packages/forms/signals/test/node/submit.spec.ts +++ b/packages/forms/signals/test/node/submit.spec.ts @@ -49,7 +49,8 @@ describe('submit', () => { params, loader: () => resolvers.promise, }), - errors: () => {}, + onSuccess: () => {}, + onError: () => {}, }); }, {injector: TestBed.inject(Injector)}, diff --git a/packages/forms/signals/test/node/validation_status.spec.ts b/packages/forms/signals/test/node/validation_status.spec.ts index d45a2555161..c01e0ea5508 100644 --- a/packages/forms/signals/test/node/validation_status.spec.ts +++ b/packages/forms/signals/test/node/validation_status.spec.ts @@ -188,7 +188,8 @@ describe('validation status', () => { setTimeout(() => r(validateValueForChild(params, undefined))), ), })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); }, {injector}, @@ -233,11 +234,12 @@ describe('validation status', () => { setTimeout(() => r(validateValueForChild(params, undefined))), ), })), - errors: (errs, {fieldOf}) => - errs.map((e) => ({ + onSuccess: (results, {fieldOf}) => + results.map((e) => ({ ...e, field: fieldOf(p.child), })), + onError: () => null, }); }, {injector}, @@ -282,11 +284,12 @@ describe('validation status', () => { setTimeout(() => r(validateValueForChild(params, undefined))), ), })), - errors: (errs, {fieldOf}) => - errs.map((e) => ({ + onSuccess: (results, {fieldOf}) => + results.map((e) => ({ ...e, field: fieldOf(p.child), })), + onError: () => null, }); }, {injector}, @@ -334,11 +337,12 @@ describe('validation status', () => { setTimeout(() => r(validateValueForChild(params, undefined))), ), })), - errors: (errs, {fieldOf}) => - errs.map((e) => ({ + onSuccess: (results, {fieldOf}) => + results.map((e) => ({ ...e, field: fieldOf(p.child), })), + onError: () => null, }); }, {injector}, @@ -383,7 +387,8 @@ describe('validation status', () => { setTimeout(() => r(validateValueForChild(params, undefined))), ), })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); }, {injector}, @@ -443,7 +448,8 @@ describe('validation status', () => { params, loader: () => new Promise((r) => setTimeout(() => r([]))), })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); }, {injector}, @@ -470,7 +476,8 @@ describe('validation status', () => { params, loader: () => promise, })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); validateAsync(p, { params: () => [], @@ -479,7 +486,8 @@ describe('validation status', () => { params, loader: () => promise2, })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); }, {injector}, @@ -514,7 +522,8 @@ describe('validation status', () => { params, loader: () => invalidPromise, })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); validateAsync(p, { params: () => [], @@ -523,7 +532,8 @@ describe('validation status', () => { params, loader: () => validPromise, })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); validateAsync(p, { params: () => [], @@ -532,7 +542,8 @@ describe('validation status', () => { params, loader: () => pendingPromise, })), - errors: (errs) => errs, + onSuccess: (results) => results, + onError: () => null, }); }, {injector},