fix(store): add setting that allows disabling dual error handling #1971

arturovt · 2023-03-01T15:35:29Z

Issue: #1965

NGXS has had an explicit error-handling mechanism from actions for the last years. However, this caused issues with ErrorHandler being called twice with the same error. The explicit error handling mechanism is required because of the NGXS action handling strategy. The default action handling process leaves the Angular zone, so NGXS actions are invoked in the <root> zone context. All caught actions must be returned to the Angular error handler.

Angular's default error-handling mechanism requires zone.js to be bundled. The forked Angular zone has an onHandleError hook which calls zone.onError.emit(error) when any error is being caught in Angular zone. Angular subscribes to onError when calls bootstrapModuleFactory:

const exceptionHandler = moduleRef.injector.get(ErrorHandler, null);

...

const subscription = ngZone.onError.subscribe({
    next: (error) => {
        exceptionHandler.handleError(error);
    }
});

This PR adds "switchers" between explicit and implicit error-handling mechanisms. We either catch them manually and delegate them to handleError or allow Angular to catch errors in its zone and delegate them to handleError.

We would use the old behavior where we catch errors explicitly to be backward-compatible and allow users to switch to the implicit behavior where zone.js is responsible for catching errors and delegating them to Angular.

docs/advanced/errors.md

markwhitfeld

I think that this is a great addition, but I'm unsure about the name.
What other names have you thought about?

markwhitfeld · 2023-03-21T19:44:11Z

@arturovt could you describe the two different modes?
The PR description gives a great overview and it describes the two modes as explicit and implicit error handling but the exact difference is unclear.

Could you summarize each mode with details of behavior under each mode?

This will go a long way to help with naming.

markwhitfeld · 2023-03-21T20:03:47Z

Upon reviewing this, is suspect that there may actually be a fix that allows this to work as expected in all cases.
I think that there is a bug as I suspected in this comment: https://github.com/ngxs/store/pull/1927/files#r1018904814
The other bug as mentioned here (where the error is reported async):
#1965 (comment)

could be to do with the Promise.resolve().then(... found here:
https://github.com/ngxs/store/pull/1927/files#diff-d211bce5ab62868a141a2954aeb1c7a4f3d3cf03dd08a8ef4f1c8496e1548f0dR32

Do you remember the motivation for this new microtask?

bundlemon · 2023-07-13T17:37:05Z

BundleMon

Files updated (1)

Status	Path	Size	Limits
❌	fesm2015/ngxs-store.js	96.12KB (+1.65KB +1.75%)	125KB / +0.5%

Unchanged files (2)

Status	Path	Size	Limits
✅	fesm2015/ngxs-store-operators.js	6.23KB	15KB / +0.5%
✅	fesm2015/ngxs-store-internals.js	3.57KB	20KB / +0.5%

Total files change +1.65KB +1.58%

Groups updated (3)

Status	Path	Size	Limits
✅	@ngxs/store(esm2015)[gzip] _{./esm2015/*/.js}	181.94KB (+1.67KB +0.93%)	+1%
❌	@ngxs/store(umd)[gzip] _{./bundles/*.umd.js}	37.02KB (+485B +1.3%)	+1%
❌	@ngxs/store(fesm2015)[gzip] _{./fesm2015/*.js}	25.41KB (+399B +1.56%)	+1%

Final result: ❌

View report in BundleMon website ➡️

Current branch size history | Target branch size history

bundlemon · 2023-07-13T17:37:08Z

BundleMon (NGXS Plugins)

Unchanged files (14)

Status	Path	Size	Limits
✅	Plugins(umd)[gzip] _{storage-plugin/bundles/ngxs-storage-plugin.um d.js}	7.94KB	+1%
✅	Plugins(umd)[gzip] _{router-plugin/bundles/ngxs-router-plugin.umd. js}	7.41KB	+1%
✅	Plugins(umd)[gzip] _{websocket-plugin/bundles/ngxs-websocket-plugi n.umd.js}	6.92KB	+1%
✅	Plugins(umd)[gzip] _{hmr-plugin/bundles/ngxs-hmr-plugin.umd.js}	6.89KB	+1%
✅	Plugins(fesm2015)[gzip] _{storage-plugin/fesm2015/ngxs-storage-plugin.j s}	3.67KB	+1%
✅	Plugins(umd)[gzip] _{form-plugin/bundles/ngxs-form-plugin.umd.js}	3.44KB	+1%
✅	Plugins(fesm2015)[gzip] _{router-plugin/fesm2015/ngxs-router-plugin.js}	3.09KB	+1%
✅	Plugins(umd)[gzip] _{devtools-plugin/bundles/ngxs-devtools-plugin. umd.js}	2.75KB	+1%
✅	Plugins(fesm2015)[gzip] _{form-plugin/fesm2015/ngxs-form-plugin.js}	2.67KB	+1%
✅	Plugins(fesm2015)[gzip] _{hmr-plugin/fesm2015/ngxs-hmr-plugin.js}	2.65KB	+1%
✅	Plugins(fesm2015)[gzip] _{websocket-plugin/fesm2015/ngxs-websocket-plug in.js}	2.59KB	+1%
✅	Plugins(umd)[gzip] _{logger-plugin/bundles/ngxs-logger-plugin.umd. js}	2.53KB	+1%
✅	Plugins(fesm2015)[gzip] _{devtools-plugin/fesm2015/ngxs-devtools-plugin .js}	2.17KB	+1%
✅	Plugins(fesm2015)[gzip] _{logger-plugin/fesm2015/ngxs-logger-plugin.js}	2.01KB	+1%

No change in files bundle size

Unchanged groups (3)

Status	Path	Size	Limits
✅	All Plugins(esm2015)[gzip] _{./-plugin/esm2015/**/.js}	108.83KB	+1%
✅	All Plugins(umd)[gzip] _{./-plugin/bundles/.umd.js}	37.89KB	+1%
✅	All Plugins(fesm2015)[gzip] _{./-plugin/fesm2015/.js}	18.85KB	+1%

Final result: ✅

View report in BundleMon website ➡️

Current branch size history | Target branch size history

nx-cloud · 2023-07-13T17:59:21Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 4a913cf. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

bundlemon · 2023-07-13T20:04:01Z

BundleMon (Integration Projects)

Files updated (2)

Status	Path	Size	Limits
✅	Main bundles(Gzip) _{hello-world-ng17/dist-integration/main.(hash) .js}	67.44KB (+41B +0.06%)	+1%
✅	Main bundles(Gzip) _{hello-world-ng16/dist-integration/main.(hash) .js}	65.94KB (+39B +0.06%)	+1%

Total files change +80B +0.06%

Final result: ✅

View report in BundleMon website ➡️

Current branch size history | Target branch size history

nx-cloud · 2024-02-16T10:02:39Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a070882. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

codeclimate · 2024-02-16T10:12:00Z

Code Climate has analyzed commit a070882 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 95.5% (0.0% change).

View more on Code Climate.

markwhitfeld · 2024-03-21T16:03:40Z

I had a discussion with @arturovt and we closed this PR in favour of another approach that will be taken in an upcoming PR.

arturovt force-pushed the fix/1965 branch from ffa4156 to 94968f7 Compare March 2, 2023 20:10

arturovt force-pushed the fix/1965 branch 4 times, most recently from e07aa28 to 5ea35e0 Compare March 20, 2023 18:20

arturovt marked this pull request as ready for review March 20, 2023 18:21

arturovt requested review from markwhitfeld, kuncevic, Carniatto, poloagustin, joaqcid and splincode as code owners March 20, 2023 18:21

Carniatto reviewed Mar 20, 2023

View reviewed changes

docs/advanced/errors.md Outdated Show resolved Hide resolved

markwhitfeld reviewed Mar 20, 2023

View reviewed changes

ngxs deleted a comment from codeclimate bot Mar 21, 2023

ngxs deleted a comment from bundlemon bot Jul 13, 2023

arturovt force-pushed the fix/1965 branch from dc9dae6 to 3894ddb Compare July 13, 2023 17:30

ngxs deleted a comment from bundlemon bot Jul 13, 2023

arturovt force-pushed the fix/1965 branch from 3894ddb to e9476d8 Compare July 13, 2023 19:56

arturovt changed the title ~~fix(store): add flag that allows using new error handling mechanism~~ fix(store): add setting that allows disabling dual error handling Jul 13, 2023

arturovt force-pushed the fix/1965 branch 2 times, most recently from 8fd2b17 to 4a913cf Compare July 13, 2023 21:18

fix(store): add setting that allows disabling dual error handling

a070882

arturovt force-pushed the fix/1965 branch from 4a913cf to a070882 Compare February 16, 2024 10:00

arturovt closed this Mar 21, 2024

arturovt deleted the fix/1965 branch March 21, 2024 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(store): add setting that allows disabling dual error handling #1971

fix(store): add setting that allows disabling dual error handling #1971

arturovt commented Mar 1, 2023 •

edited

Loading

markwhitfeld left a comment

markwhitfeld commented Mar 21, 2023 •

edited

Loading

markwhitfeld commented Mar 21, 2023

bundlemon bot commented Jul 13, 2023 •

edited

Loading

bundlemon bot commented Jul 13, 2023 •

edited

Loading

nx-cloud bot commented Jul 13, 2023 •

edited

Loading

bundlemon bot commented Jul 13, 2023 •

edited

Loading

nx-cloud bot commented Feb 16, 2024 •

edited

Loading

codeclimate bot commented Feb 16, 2024

markwhitfeld commented Mar 21, 2024

fix(store): add setting that allows disabling dual error handling #1971

fix(store): add setting that allows disabling dual error handling #1971

Conversation

arturovt commented Mar 1, 2023 • edited Loading

markwhitfeld left a comment

Choose a reason for hiding this comment

markwhitfeld commented Mar 21, 2023 • edited Loading

markwhitfeld commented Mar 21, 2023

bundlemon bot commented Jul 13, 2023 • edited Loading

BundleMon

bundlemon bot commented Jul 13, 2023 • edited Loading

BundleMon (NGXS Plugins)

nx-cloud bot commented Jul 13, 2023 • edited Loading

☁️ Nx Cloud Report

bundlemon bot commented Jul 13, 2023 • edited Loading

BundleMon (Integration Projects)

nx-cloud bot commented Feb 16, 2024 • edited Loading

☁️ Nx Cloud Report

codeclimate bot commented Feb 16, 2024

markwhitfeld commented Mar 21, 2024

arturovt commented Mar 1, 2023 •

edited

Loading

markwhitfeld commented Mar 21, 2023 •

edited

Loading

bundlemon bot commented Jul 13, 2023 •

edited

Loading

bundlemon bot commented Jul 13, 2023 •

edited

Loading

nx-cloud bot commented Jul 13, 2023 •

edited

Loading

bundlemon bot commented Jul 13, 2023 •

edited

Loading

nx-cloud bot commented Feb 16, 2024 •

edited

Loading