Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Effects): stringify action on invalid action #1219

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
61 changes: 58 additions & 3 deletions modules/effects/spec/effect_sources.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ describe('EffectSources', () => {
const b = { type: 'From Source B' };
const c = { type: 'From Source C that completes' };
const d = { not: 'a valid action' };
const e = undefined;
const f = null;

let circularRef = {} as any;
circularRef.circularRef = circularRef;
const g = { circularRef };

const error = new Error('An Error');

class SourceA {
Expand All @@ -54,10 +61,22 @@ describe('EffectSources', () => {
}

class SourceE {
@Effect() e$ = throwError(error);
@Effect() e$ = alwaysOf(e);
}

class SourceF {
@Effect() f$ = alwaysOf(f);
}

class SourceG {
@Effect() g$ = alwaysOf(g);
}

class SourceError {
@Effect() e$ = throwError(error);
}

class SourceH {
@Effect() empty = of('value');
@Effect()
never = timer(50, getTestScheduler() as any).pipe(map(() => 'update'));
Expand Down Expand Up @@ -90,12 +109,48 @@ describe('EffectSources', () => {

toActions(sources$).subscribe();

expect(mockErrorReporter.handleError).toHaveBeenCalled();
expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
new Error(
'Effect "SourceD.d$" dispatched an invalid action: {"not":"a valid action"}'
)
);
});

it('should report an error if an effect dispatches an `undefined`', () => {
const sources$ = of(new SourceE());

toActions(sources$).subscribe();

expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
new Error('Effect "SourceE.e$" dispatched an invalid action: undefined')
);
});

it('should report an error if an effect dispatches a `null`', () => {
const sources$ = of(new SourceF());

toActions(sources$).subscribe();

expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
new Error('Effect "SourceF.f$" dispatched an invalid action: null')
);
});

it(`should not break when the action in the error message can't be stringified`, () => {
const sources$ = of(new SourceG());

toActions(sources$).subscribe();

expect(mockErrorReporter.handleError).toHaveBeenCalledWith(
new Error(
'Effect "SourceG.g$" dispatched an invalid action: [object Object]'
)
);
});

it('should not complete the group if just one effect completes', () => {
const sources$ = cold('g', {
g: new SourceG(),
g: new SourceH(),
});
const expected = cold('a----b-----', { a: 'value', b: 'update' });

Expand Down
10 changes: 9 additions & 1 deletion modules/effects/src/effect_notification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function reportInvalidActions(
new Error(
`Effect ${getEffectName(
output
)} dispatched an invalid action: ${action}`
)} dispatched an invalid action: ${stringify(action)}`
)
);
}
Expand All @@ -57,3 +57,11 @@ function getEffectName({

return `"${sourceName}.${propertyName}${isMethod ? '()' : ''}"`;
}

function stringify(action: Action | null | undefined) {
try {
return JSON.stringify(action);
} catch {
return action;
}
}