Zone-JS Error Forwading issue

[saschaarthur]

Environment
Provide version numbers for the following components (information can be retrieved by running tns info in your project folder or by inspecting the package.json of the project):

• CLI: 7.0.10

Describe the bug
Errors are not forwarded to the Error Logger (tracer), instead those error ares written to console.error(). This affects production builds, and its hard to find context information without stacktrace.

I already have an patch ready for @nativescript/zone-js/zone-nativescript.js but the build system behind this is kind of crazy if theres any(?) i only find hardly outdated branches and not even source code for the given zone-nativescript.js .. The files in @nativescript/zone-js are also at least duplicated, also theres .tar.gz stuff laying arround.. Lets clean this up please.. Please someone point me where to start..

To Reproduce
Create NS-Angular project
Throw an exception for example throw new Error('test'); in constructor

Expected behavior
should be forwarded to Tracer.error instead of console.error(x);

so we can trace it down with something like

const errorHandler: TraceErrorHandler = {
    handlerError(err: any): void {
        StaticErrorLogger.getInstance().logTypeUnsafeException(err);
    }
};
traceModule.Trace.setErrorHandler(errorHandler);

[saschaarthur]

@NathanaelA @NathanWalker

I guess you guys maybe know the best. This patch here should be applied somewhat to the build process of the @nativescript/zone-js package. Afterwards none of the errors (thrown in nativescript) are ignored anymore but instead forwarded to the traceModul (which i guess is/should be the wanted behaviour?).

diff --git a/node_modules/@nativescript/zone-js/zone-nativescript.js b/node_modules/@nativescript/zone-js/zone-nativescript.js
index 39a5b55..8178344 100644
--- a/node_modules/@nativescript/zone-js/zone-nativescript.js
+++ b/node_modules/@nativescript/zone-js/zone-nativescript.js
@@ -5,6 +5,8 @@
 * Use of this source code is governed by an MIT-style license that can be
 * found in the LICENSE file at https://angular.io/license
 */
+import * as trace from '@nativescript/core/trace';
+
 (function (global, factory) {
 	typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
 	typeof define === 'function' && define.amd ? define(factory) :
@@ -673,9 +675,11 @@ Zone.__load_patch('ZoneAwarePromise', function (global, Zone, api) {
         if (api.showUncaughtError()) {
             var rejection = e && e.rejection;
             if (rejection) {
+                trace.Trace.error(rejection);
                 console.error('Unhandled Promise rejection:', rejection instanceof Error ? rejection.message : rejection, '; Zone:', e.zone.name, '; Task:', e.task && e.task.source, '; Value:', rejection, rejection instanceof Error ? rejection.stack : undefined);
             }
             else {
+                trace.Trace.error(e);
                 console.error(e);
             }
         }
@@ -706,8 +710,10 @@ Zone.__load_patch('ZoneAwarePromise', function (global, Zone, api) {
             if (handler && typeof handler === 'function') {
                 handler.call(this, e);
             }
+            trace.Trace.error(e);
         }
         catch (err) {
+            trace.Trace.error(err);
         }
     }
     function isThenable(value) {
@@ -855,6 +861,7 @@ Zone.__load_patch('ZoneAwarePromise', function (global, Zone, api) {
                 }
             }
             catch (err) {
+                trace.Trace.error(err);
             }
             promise[symbolState] = REJECTED;
             for (var i = 0; i < _uncaughtPromiseErrors.length; i++) {
@@ -1337,6 +1344,7 @@ Zone.__load_patch('Error', function (global, Zone, api) {
             }
             catch (e) {
                 // ignore as some browsers don't allow overriding of stack
+                trace.Trace.error(e);
             }
         }
         if (this instanceof NativeError && this.constructor != NativeError) {
@@ -1350,6 +1358,7 @@ Zone.__load_patch('Error', function (global, Zone, api) {
                     }
                     catch (e) {
                         // ignore the assignment in case it is a setter and it throws.
+                        trace.Trace.error(e);
                     }
                 }
             });

I even did one step more and cleaned up the whole package with this patch here (deleteing all unused/duplicated files..): https://pastebin.com/w17vscZS.

Again: i would create a PR for it but i dont find the place for this package it seems somehow made manually. Thats why it would be awesome to have an full overview of nativescript and how its build with the full process of a working CI/CD pipeline NativeScript/android#1623.

[edusperoni]

IIRC zone code isn't in any repo. It was on this repo before as a single js file, which was bad already tbh.

That said we're moving away from a custom zone and aiming to use zone.js without any hard patches, only what their API allows us. Current zone.js allows us to define custom error handlers already, and most of nativescript's polyfills are sufficient for it to work without any workarounds.

[saschaarthur]

2. I have no idea on the trace, Nathan probably would be better to speak to that as Zones is Angular specific, are you hooking into the trace system to capture things; is that why you want these errors//warnings forwarded to the Trace system?

Yes we are facing issues which we dont know the source where they are coming from. I figured out that we didnt implement the trace error handler, but the source of the crashes seems to be inside of the promises of this Zone stuff.

So i was expecting that those are forwarded to the trace system and not ending just in console.err

[saschaarthur]

We were able - with the given patch above, to trace down the source of the issue. Its seems its simple coming from the snackbar.

nstudio/nativescript-snackbar#20

Edit: code seems to be there, pipeline still fails somehow on this repo.

[edusperoni]

With the new nativescript-angular 12 release (https://blog.nativescript.org/nativescript-angular-12/index.html), we no longer use a custom zone.js so you can now override the unhandled promise rejection as you best see fit using Zone.js's own API:

Zone[Zone.__symbol__['ignoreConsoleErrorUncaughtError']] = true;
Zone[Zone.__symbol__['unhandledPromiseRejectionHandler']] = (e) => {
  // handle your rejection here
};

[hlovdal]

Notice that should be Zone.__symbol__('...').