Skip to content

Commit 05eb028

Browse files
Humberdatscott
authored andcommitted
fix(core): narrow error type for resources API (angular#61441)
`Resource.error` used to return `unknown`. Now it's `Error | undefined`. For non-`Error` types they are encapsulated with the `Error` type. PR Close angular#61441
1 parent 07811dd commit 05eb028

8 files changed

Lines changed: 54 additions & 24 deletions

File tree

goldens/public-api/core/index.api.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,7 +1607,7 @@ export function resolveForwardRef<T>(type: T): T;
16071607

16081608
// @public
16091609
export interface Resource<T> {
1610-
readonly error: Signal<unknown>;
1610+
readonly error: Signal<Error | undefined>;
16111611
hasValue(): this is Resource<Exclude<T, undefined>>;
16121612
readonly isLoading: Signal<boolean>;
16131613
readonly status: Signal<ResourceStatus>;
@@ -1657,7 +1657,7 @@ export type ResourceStreamingLoader<T, R> = (param: ResourceLoaderParams<R>) =>
16571657
export type ResourceStreamItem<T> = {
16581658
value: T;
16591659
} | {
1660-
error: unknown;
1660+
error: Error;
16611661
};
16621662

16631663
// @public

packages/common/http/src/resource.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
type ValueEqualityFn,
2020
ɵRuntimeError,
2121
ɵRuntimeErrorCode,
22+
ɵencapsulateResourceError as encapsulateResourceError,
2223
} from '@angular/core';
2324
import type {Subscription} from 'rxjs';
2425

@@ -344,7 +345,7 @@ class HttpResourceImpl<T>
344345
try {
345346
send({value: parse ? parse(event.body) : (event.body as T)});
346347
} catch (error) {
347-
send({error});
348+
send({error: encapsulateResourceError(error)});
348349
}
349350
break;
350351
case HttpEventType.DownloadProgress:

packages/core/rxjs-interop/src/rx_resource.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
ɵRuntimeErrorCode,
1919
} from '../../src/core';
2020
import {Observable, Subscription} from 'rxjs';
21+
import {encapsulateResourceError} from '../../src/resource/resource';
2122

2223
/**
2324
* Like `ResourceOptions` but uses an RxJS-based `loader`.
@@ -61,11 +62,11 @@ export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T |
6162
params.abortSignal.addEventListener('abort', onAbort);
6263

6364
// Start off stream as undefined.
64-
const stream = signal<{value: T} | {error: unknown}>({value: undefined as T});
65-
let resolve: ((value: Signal<{value: T} | {error: unknown}>) => void) | undefined;
66-
const promise = new Promise<Signal<{value: T} | {error: unknown}>>((r) => (resolve = r));
65+
const stream = signal<{value: T} | {error: Error}>({value: undefined as T});
66+
let resolve: ((value: Signal<{value: T} | {error: Error}>) => void) | undefined;
67+
const promise = new Promise<Signal<{value: T} | {error: Error}>>((r) => (resolve = r));
6768

68-
function send(value: {value: T} | {error: unknown}): void {
69+
function send(value: {value: T} | {error: Error}): void {
6970
stream.set(value);
7071
resolve?.(stream);
7172
resolve = undefined;
@@ -82,8 +83,8 @@ export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T |
8283

8384
sub = streamFn(params).subscribe({
8485
next: (value) => send({value}),
85-
error: (error) => {
86-
send({error});
86+
error: (error: unknown) => {
87+
send({error: encapsulateResourceError(error)});
8788
params.abortSignal.removeEventListener('abort', onAbort);
8889
},
8990
complete: () => {

packages/core/rxjs-interop/test/rx_resource_spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ describe('rxResource()', () => {
7474
expect(res.value()).toBe(3);
7575

7676
response.error('fail');
77-
expect(res.error()).toBe('fail');
77+
expect(res.error()).toEqual(jasmine.objectContaining({cause: 'fail'}));
78+
expect(res.error()!.message).toContain('Resource');
7879
});
7980
});
8081

packages/core/src/core_private_export.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ export {
162162
disableProfiling as ɵdisableProfiling,
163163
} from './profiler';
164164

165-
export {ResourceImpl as ɵResourceImpl} from './resource/resource';
165+
export {
166+
ResourceImpl as ɵResourceImpl,
167+
encapsulateResourceError as ɵencapsulateResourceError,
168+
} from './resource/resource';
166169

167170
export {getClosestComponentName as ɵgetClosestComponentName} from './internal/get_closest_component_name';

packages/core/src/resource/api.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export interface Resource<T> {
5959
/**
6060
* When in the `error` state, this returns the last known error from the `Resource`.
6161
*/
62-
readonly error: Signal<unknown>;
62+
readonly error: Signal<Error | undefined>;
6363

6464
/**
6565
* Whether this resource is loading a new value (or reloading the existing one).
@@ -225,4 +225,4 @@ export type ResourceOptions<T, R> = PromiseResourceOptions<T, R> | StreamingReso
225225
/**
226226
* @experimental
227227
*/
228-
export type ResourceStreamItem<T> = {value: T} | {error: unknown};
228+
export type ResourceStreamItem<T> = {value: T} | {error: Error};

packages/core/src/resource/resource.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ type WrappedRequest = {request: unknown; reload: number};
101101
abstract class BaseWritableResource<T> implements WritableResource<T> {
102102
readonly value: WritableSignal<T>;
103103
abstract readonly status: Signal<ResourceStatus>;
104-
abstract readonly error: Signal<unknown>;
104+
abstract readonly error: Signal<Error | undefined>;
105+
105106
abstract reload(): boolean;
106107

107108
constructor(value: Signal<T>) {
@@ -346,7 +347,7 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
346347
extRequest,
347348
status: 'resolved',
348349
previousStatus: 'error',
349-
stream: signal({error: err}),
350+
stream: signal({error: encapsulateResourceError(err)}),
350351
});
351352
} finally {
352353
// Resolve the pending task now that the resource has a value.
@@ -381,7 +382,7 @@ function getLoader<T, R>(options: ResourceOptions<T, R>): ResourceStreamingLoade
381382
try {
382383
return signal({value: await options.loader(params)});
383384
} catch (err) {
384-
return signal({error: err});
385+
return signal({error: encapsulateResourceError(err)});
385386
}
386387
};
387388
}
@@ -409,3 +410,22 @@ function projectStatusOfState(state: ResourceState<unknown>): ResourceStatus {
409410
function isResolved<T>(state: ResourceStreamItem<T>): state is {value: T} {
410411
return (state as {error: unknown}).error === undefined;
411412
}
413+
414+
export function encapsulateResourceError(error: unknown): Error {
415+
if (error instanceof Error) {
416+
return error;
417+
}
418+
419+
return new ResourceWrappedError(error);
420+
}
421+
422+
class ResourceWrappedError extends Error {
423+
constructor(error: unknown) {
424+
super(
425+
ngDevMode
426+
? `Resource returned an error that's not an Error instance: ${String(error)}. Check this error's .cause for the actual error.`
427+
: String(error),
428+
{cause: error},
429+
);
430+
}
431+
}

packages/core/test/resource/resource_spec.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class MockEchoBackend<T> extends MockBackend<T, T> {
7171

7272
class MockResponseCountingBackend extends MockBackend<number, string> {
7373
counter = 0;
74+
7475
override prepareResponse(request: number) {
7576
return request + ':' + this.counter++;
7677
}
@@ -145,7 +146,10 @@ describe('resource', () => {
145146
expect(echoResource.isLoading()).toBeFalse();
146147
expect(echoResource.hasValue()).toBeFalse();
147148
expect(echoResource.value()).toEqual(undefined);
148-
expect(echoResource.error()).toBe('Something went wrong....');
149+
expect(echoResource.error()).toEqual(
150+
jasmine.objectContaining({cause: 'Something went wrong....'}),
151+
),
152+
expect(echoResource.error()!.message).toContain('Resource');
149153
});
150154

151155
it('should expose errors on reload', async () => {
@@ -608,18 +612,18 @@ describe('resource', () => {
608612
it('should error via error()', async () => {
609613
const appRef = TestBed.inject(ApplicationRef);
610614
const res = resource({
611-
stream: async () => signal({error: 'fail'}),
615+
stream: async () => signal({error: new Error('fail')}),
612616
injector: TestBed.inject(Injector),
613617
});
614618

615619
await appRef.whenStable();
616620
expect(res.status()).toBe('error');
617-
expect(res.error()).toBe('fail');
621+
expect(res.error()).toEqual(new Error('fail'));
618622
});
619623

620624
it('should transition across streamed states', async () => {
621625
const appRef = TestBed.inject(ApplicationRef);
622-
const stream = signal<{value: number} | {error: unknown}>({value: 1});
626+
const stream = signal<{value: number} | {error: Error}>({value: 1});
623627

624628
const res = resource({
625629
stream: async () => stream,
@@ -633,16 +637,16 @@ describe('resource', () => {
633637
stream.set({value: 3});
634638
expect(res.value()).toBe(3);
635639

636-
stream.set({error: 'fail'});
637-
expect(res.error()).toBe('fail');
640+
stream.set({error: new Error('fail')});
641+
expect(res.error()).toEqual(new Error('fail'));
638642

639643
stream.set({value: 4});
640644
expect(res.value()).toBe(4);
641645
});
642646

643647
it('should not accept new values/errors after a request is cancelled', async () => {
644648
const appRef = TestBed.inject(ApplicationRef);
645-
const stream = signal<{value: number} | {error: unknown}>({value: 0});
649+
const stream = signal<{value: number} | {error: Error}>({value: 0});
646650
const request = signal(1);
647651
const res = resource({
648652
params: request,
@@ -666,7 +670,7 @@ describe('resource', () => {
666670
// The previous set/error functions should no longer result in changes to the resource.
667671
stream.set({value: 2});
668672
expect(res.value()).toBe(undefined);
669-
stream.set({error: 'fail'});
673+
stream.set({error: new Error('fail')});
670674
expect(res.value()).toBe(undefined);
671675
});
672676

0 commit comments

Comments
 (0)