Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(core): getting resource value throws an error instead of returnin…
…g undefined

When there is an underlying error state it would not be possible to swallow the error with:
`computed(() => res.value()?.inner);`
  • Loading branch information
Humberd authored and alxhub committed May 21, 2025
commit e60e3229eaf00b79b47a755c66d50f75b56e9333
4 changes: 2 additions & 2 deletions packages/core/src/resource/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export type ResourceStatus = 'idle' | 'error' | 'loading' | 'reloading' | 'resol
*/
export interface Resource<T> {
/**
* The current value of the `Resource`, or `undefined` if there is no current value.
* The current value of the `Resource`, or throws an error if the resource is in an error state.
*/
readonly value: Signal<T>;

Expand Down Expand Up @@ -167,7 +167,7 @@ export interface BaseResourceOptions<T, R> {

/**
* The value which will be returned from the resource when a server value is unavailable, such as
* when the resource is still loading, or in an error state.
* when the resource is still loading.
*/
defaultValue?: NoInfer<T>;

Expand Down
48 changes: 45 additions & 3 deletions packages/core/src/resource/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ import {PendingTasks} from '../pending_tasks';
import {linkedSignal} from '../render3/reactivity/linked_signal';
import {DestroyRef} from '../linker/destroy_ref';

/**
* Whether a `Resource.value()` should throw an error when the resource is in the error state.
*
* This internal flag is being used to gradually roll out this behavior.
*/
const RESOURCE_VALUE_THROWS_ERRORS_DEFAULT = true;

/**
* Constructs a `Resource` that projects a reactive request to an asynchronous operation defined by
* a loader function, which exposes the result of the loading operation via signals.
Expand Down Expand Up @@ -72,6 +79,7 @@ export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T |
options.defaultValue,
options.equal ? wrapEqualityFn(options.equal) : undefined,
options.injector ?? inject(Injector),
RESOURCE_VALUE_THROWS_ERRORS_DEFAULT,
);
}

Expand Down Expand Up @@ -114,13 +122,22 @@ abstract class BaseWritableResource<T> implements WritableResource<T> {

abstract set(value: T): void;

private readonly isError = computed(() => this.status() === 'error');

update(updateFn: (value: T) => T): void {
this.set(updateFn(untracked(this.value)));
}

readonly isLoading = computed(() => this.status() === 'loading' || this.status() === 'reloading');

hasValue(): this is ResourceRef<Exclude<T, undefined>> {
// Note: we specifically read `isError()` instead of `status()` here to avoid triggering
// reactive consumers which read `hasValue()`. This way, if `hasValue()` is used inside of an
// effect, it doesn't cause the effect to rerun on every status change.
if (this.isError()) {
return false;
}

return this.value() !== undefined;
}

Expand Down Expand Up @@ -154,17 +171,31 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
constructor(
request: () => R,
private readonly loaderFn: ResourceStreamingLoader<T, R>,
private readonly defaultValue: T,
defaultValue: T,
private readonly equal: ValueEqualityFn<T> | undefined,
injector: Injector,
throwErrorsFromValue: boolean = RESOURCE_VALUE_THROWS_ERRORS_DEFAULT,
) {
super(
// Feed a computed signal for the value to `BaseWritableResource`, which will upgrade it to a
// `WritableSignal` that delegates to `ResourceImpl.set`.
computed(
() => {
const streamValue = this.state().stream?.();
return streamValue && isResolved(streamValue) ? streamValue.value : this.defaultValue;

if (!streamValue) {
return defaultValue;
}

if (!isResolved(streamValue)) {
if (throwErrorsFromValue) {
throw new ResourceValueError(this.error()!);
} else {
return defaultValue;
}
}

return streamValue.value;
},
{equal},
),
Expand Down Expand Up @@ -401,7 +432,7 @@ function projectStatusOfState(state: ResourceState<unknown>): ResourceStatus {
case 'loading':
return state.extRequest.reload === 0 ? 'loading' : 'reloading';
case 'resolved':
return isResolved(untracked(state.stream!)) ? 'resolved' : 'error';
return isResolved(state.stream!()) ? 'resolved' : 'error';
default:
return state.status;
}
Expand All @@ -419,6 +450,17 @@ export function encapsulateResourceError(error: unknown): Error {
return new ResourceWrappedError(error);
}

class ResourceValueError extends Error {
constructor(error: Error) {
super(
ngDevMode
? `Resource is currently in an error state (see Error.cause for details): ${error.message}`
: error.message,
{cause: error},
);
}
}

class ResourceWrappedError extends Error {
constructor(error: unknown) {
super(
Expand Down
136 changes: 127 additions & 9 deletions packages/core/test/resource/resource_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

import {
ApplicationRef,
computed,
createEnvironmentInjector,
effect,
EnvironmentInjector,
Injector,
resource,
Expand Down Expand Up @@ -140,16 +142,18 @@ describe('resource', () => {
});

TestBed.tick();
await backend.reject(requestParam, 'Something went wrong....');
await backend.reject(requestParam, new Error('Something went wrong....'));

expect(echoResource.status()).toBe('error');
expect(echoResource.isLoading()).toBeFalse();
expect(echoResource.hasValue()).toBeFalse();
expect(echoResource.value()).toEqual(undefined);
expect(echoResource.error()).toEqual(
jasmine.objectContaining({cause: 'Something went wrong....'}),
),
expect(echoResource.error()!.message).toContain('Resource');

const err = extractError(() => echoResource.value())!;
expect(err).not.toBeUndefined();
expect(err instanceof Error).toBeTrue();
expect(err!.message).toContain('Resource');
expect(err.cause).toEqual(new Error('Something went wrong....'));
expect(echoResource.error()).toEqual(new Error('Something went wrong....'));
});

it('should expose errors on reload', async () => {
Expand Down Expand Up @@ -183,8 +187,109 @@ describe('resource', () => {
expect(echoResource.status()).toBe('error');
expect(echoResource.isLoading()).toBeFalse();
expect(echoResource.hasValue()).toBeFalse();
expect(echoResource.value()).toEqual(undefined);
const err = extractError(() => echoResource.value())!;
expect(err).not.toBeUndefined();
expect(err.message).toContain('Resource');
expect(err.message).toContain('KO');
expect(err.cause).toEqual(new Error('KO'));
expect(echoResource.error()).toEqual(Error('KO'));

counter.update((value) => value + 1);
TestBed.tick();
await backend.flush();

expect(echoResource.status()).toBe('resolved');
expect(echoResource.isLoading()).toBeFalse();
expect(echoResource.hasValue()).toBeTrue();
expect(echoResource.value()).toEqual('ok');
expect(echoResource.error()).toBe(undefined);
});

it('should not trigger consumers on every status change via hasValue()', async () => {
const params = signal<number | undefined>(undefined);
const testResource = resource({
params,
// Hanging promise, never resolves
loader: () => new Promise(() => {}),
injector: TestBed.inject(Injector),
});

let effectRuns = 0;
effect(
() => {
testResource.hasValue();
effectRuns++;
},
{injector: TestBed.inject(Injector)},
);

TestBed.tick();
// Params are undefined, status should be idle.
expect(testResource.status()).toBe('idle');
// Effect should run the first time.
expect(effectRuns).toBe(1);

// Set params, causing the stauts to become 'loading'.
params.set(0);
expect(testResource.status()).toBe('loading');
TestBed.tick();
// The effect should not rerun.
expect(effectRuns).toBe(1);
});

it('should update computed signals', async () => {
const backend = new MockEchoBackend();
const counter = signal(0);
const echoResource = resource({
params: () => ({counter: counter()}),
loader: (params) => {
if (params.params.counter % 2 === 0) {
return Promise.resolve(params.params.counter);
} else {
throw new Error('KO');
}
},
injector: TestBed.inject(Injector),
});
const computedValue = computed(() => {
if (!echoResource.hasValue()) {
return -1;
}
return echoResource.value();
});

TestBed.tick();
await backend.flush();

expect(echoResource.status()).toBe('resolved');
expect(echoResource.hasValue()).toBeTrue();
expect(echoResource.value()).toEqual(0);
expect(computedValue()).toEqual(0);
expect(echoResource.error()).toBe(undefined);

counter.update((value) => value + 1);
TestBed.tick();
await backend.flush();

expect(echoResource.status()).toBe('error');
expect(echoResource.hasValue()).toBeFalse();
const err = extractError(() => echoResource.value())!;
expect(err).not.toBeUndefined();
expect(err.message).toContain('Resource');
expect(err.message).toContain('KO');
expect(err.cause).toEqual(new Error('KO'));
expect(computedValue()).toEqual(-1);
expect(echoResource.error()).toEqual(Error('KO'));

counter.update((value) => value + 1);
TestBed.tick();
await backend.flush();

expect(echoResource.status()).toBe('resolved');
expect(echoResource.hasValue()).toBeTrue();
expect(echoResource.value()).toEqual(2);
expect(computedValue()).toEqual(2);
expect(echoResource.error()).toBe(undefined);
});

it('should respond to a request that changes while loading', async () => {
Expand Down Expand Up @@ -231,7 +336,7 @@ describe('resource', () => {
expect(res.value()).toBe(1);
});

it('should return a default value if provided', async () => {
it('should throw an error when getting a value even when provided with a default value', async () => {
const DEFAULT: string[] = [];
const request = signal(0);
const res = resource({
Expand All @@ -256,7 +361,11 @@ describe('resource', () => {
request.set(2);
await TestBed.inject(ApplicationRef).whenStable();
expect(res.error()).not.toBeUndefined();
expect(res.value()).toBe(DEFAULT);
const err = extractError(() => res.value())!;
expect(err).not.toBeUndefined();
expect(err.message).toContain('Resource');
expect(err.message).toContain('err');
expect(err.cause).toEqual(new Error('err'));
});

it('should _not_ load if the request resolves to undefined', () => {
Expand Down Expand Up @@ -731,3 +840,12 @@ describe('resource', () => {
function flushMicrotasks(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 0));
}

function extractError(fn: () => unknown): Error | undefined {
try {
fn();
return undefined;
} catch (err: unknown) {
return err as Error;
}
}