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

[BUG] Application Insights reports 'not_specified' to Azure when tracking unhandled browser exception #1940

Closed
PieterWillemen opened this issue Oct 31, 2022 · 12 comments
Assignees
Milestone

Comments

@PieterWillemen
Copy link

PieterWillemen commented Oct 31, 2022

Description/Screenshot
Application Insights has automatic exception tracking enabled by default.
However, it seems to assume that the exception is always of type Error.
If the exception is simply a string, the trackException itself fails, throws an exception with message 'not_specified'.

Steps to Reproduce
Here is my setup:

import { DebugPlugin } from '@microsoft/applicationinsights-debugplugin-js';
import { ApplicationInsights } from '@microsoft/applicationinsights-web'

// Initialize AppInsights
const toTrack = [
	'trackEvent',
	'trackPageView',
	'trackPageViewPerformance',
	'trackException',
	'trackTrace',
	'trackMetric',
	'trackDependencyData',
	'throwInternal',        // called when a message is logged internally
	'logInternalMessage',   // called when a message is logged internally
	'triggerSend',          // called when data is queued to be sent to the server
	'_sender',              // called when data is sent to the server
];
const debugPluginInstance = new DebugPlugin();
const applocation = this.AppLocation;
const pageName = EAppLocationPage[applocation.Page];
const appInsights = new ApplicationInsights({
	config: {
		connectionString: (window as any).appInsightsConfig.connectionString,
		extensions: [debugPluginInstance],
		extensionConfig: {
			[DebugPlugin.identifier]: {
				trackers: toTrack
			}
		},
		disableExceptionTracking: false,
		enableDebug: false,
		loggingLevelConsole: 2, //0: off, 1: Critical errors only, 2: Everything(errors & warnings)
		loggingLevelTelemetry: 2, //0: off, 1: Critical errors only, 2: Everything(errors & warnings)
		enableRequestHeaderTracking: true,
		enableResponseHeaderTracking: true,
		correlationHeaderExcludedDomains: ['*.queue.core.windows.net']
	}
});
appInsights.setAuthenticatedUserContext((window as any).appInsightsConfig.authenticatedUserContext);
appInsights.addTelemetryInitializer((x) => {
	x.tags!['ai.operation.name'] = 'Full_Load/' + pageName;
});
appInsights.loadAppInsights();

When I throw a dummy exception with simply a string message in the console to test AI's auto trackException, this is what AI does:
(Stacktrace shown by using the @microsoft/applicationinsights-debugplugin-js plugin.)

image

It seems that AI then itself fails, as it does not expect a string, but an Error.
This it what is then reported to Azure:

image

AnalyticsPlugin/</_self[_DynamicConstants__WEBPACK_IMPORTED_MODULE_1__._DYN_SEND_EXCEPTION_INTER13]@https://localhost:44385/js/ViewModels/patientsViewModel.min.js?v=PVKRkSfmexWSsx2UaNtVhIFEkwUYJoTDiuQ6OoT7Tyw:15684:224
AnalyticsPlugin/</_self.trackException@https://localhost:44385/js/ViewModels/patientsViewModel.min.js?v=PVKRkSfmexWSsx2UaNtVhIFEkwUYJoTDiuQ6OoT7Tyw:15703:138
dynProtoProxy@https://localhost:44385/js/ViewModels/patientsViewModel.min.js?v=PVKRkSfmexWSsx2UaNtVhIFEkwUYJoTDiuQ6OoT7Tyw:33706:29
_createFunctionHook/<@https://localhost:44385/js/ViewModels/patientsViewModel.min.js?v=PVKRkSfmexWSsx2UaNtVhIFEkwUYJoTDiuQ6OoT7Tyw:26241:119
_createProxyFunction/<@https://localhost:44385/js/ViewModels/patientsViewModel.min.js?v=PVKRkSfmexWSsx2UaNtVhIFEkwUYJoTDiuQ6OoT7Tyw:25946:112
@debugger eval code:1:20

If I throw an exception in code of type Error and catch it myself to manually call trackException(),

try {
	throw new Error('TESTPIETER FULL');
}
catch (error) {
	if (error instanceof Error) {
		appInsights.trackException({
			exception: error
		});
	} else {
		appInsights.trackException({
			exception: new Error(error as string)
		});
	}
}

then I see the following:

image

And the error is correctly reported to Azure:

image

So, to me, it seems that AI exception tracking does not work correctly out of the box.
Currently, I would need to override the current global exception handler of the browser, to convert a string exception to an Error and call trackException() manually.
This should be done by AI itself out of the box.

  • OS/Browser: Any browser
  • SDK Version [e.g. 22]: 2.8.9
  • How you initialized the SDK: npm
@PieterWillemen
Copy link
Author

PieterWillemen commented Oct 31, 2022

I found a related issue here, where it is recommended to use the correct syntax when calling trackException().
#1006

But the actual issue is not solved, i.e. AI itself calls trackException() with a string.

@PieterWillemen
Copy link
Author

This issue also rises here:
OfficeDev/microsoft-teams-library-js#577

@MSNev MSNev added the investigation required Further investigation or discussions required label Oct 31, 2022
@MSNev
Copy link
Collaborator

MSNev commented Oct 31, 2022

We "should" be handling string "exceptions" and we have specific tests and code to handle this.

But at least on looking at the code it does appear that when using the snippet a "string" won't necessarily be handled (it falls back to window.event so it will work on some browsers).

For the scenario provided (using NPM) this should be working as it should come through this hook for the window.onerror https://github.com/microsoft/ApplicationInsights-JS/blob/master/extensions/applicationinsights-analytics-js/src/JavaScriptSDK/AnalyticsPlugin.ts#L699-L706 so it should "not" be calling trackException with a string value (the snippet onerror does look like this will happen)

@PieterWillemen
Copy link
Author

PieterWillemen commented Nov 2, 2022

This is what Application Insights reports as browser exceptions on our production environment:
image

As you can see on row 1, 3 and 6 'not_specified' is being reported.
Some errors like Uncaught TypeError or Uncaught SyntaxError are being successfully reported.

@MSNev MSNev self-assigned this Nov 16, 2022
@MSNev
Copy link
Collaborator

MSNev commented Nov 16, 2022

Just a note to inform that this issue is not forgotten, but due to scheduling conflicts, release lockdowns and holidays it's probably going to be early next year (2023) before I can carve out a decent amount of time to understand why strings are not always supported... I suspect that we will need to move the current string detecting code a little deeper in the stack.

@MSNev MSNev modified the milestones: 2.x.x (Next), 2.8.10 Jan 31, 2023
@MSNev
Copy link
Collaborator

MSNev commented Jan 31, 2023

Allocating to the next release, but there is a risk (depending on the size of any required change) that it may not make this release.

@PieterWillemen
Copy link
Author

Great, thanks for following up!

@MSNev
Copy link
Collaborator

MSNev commented Jan 31, 2023

Ok, I've tried 4 scenarios

  • Just call throw "string error" -> resulted in the window.onerror to get called (Chromium based Edge) with an Error with the message set as the string -- works
  • Directly call trackException(theError: IExceptionTelemetry) with a string as the exception value -- works when called directly or in a catch with the caught string. See here for the definition of the trackException arguments
  • Directly call trackException("string value") -- resulting in what looks like your scenario Error being reported in the sendExceptionInternal as it wraps the provided first object (which is supposed to be an IExceptionTelemetry object) in an error -- This is considered to be an invalid usage of the API
    <button onclick="javascript: throw 'String Error'">Throw String Exception</button>
    <button onclick="javascript: appInsights.trackException({ exception: 'String Error' })"">trackException with String as the exception</button>
    <button onclick="javascript: try { throw 'String Error'; } catch(e) { appInsights.trackException({exception: e})}">trackException with try / catch</button>
    <button onclick="javascript: appInsights.trackException('String Error')"">trackException with String</button>

So while I could instead use our helper dumpObj() function to dump the object as theError instead of creating a new Error instance, there could still be other issues as the rest of the code is still attempting to use the first argument as an iExceptionTelemetry instance...

So you "should" be able to just use the following

try {
	throw 'TESTPIETER FULL';
}
catch (error) {
   appInsights.trackException({
	exception: error
   });
}

or just let the exception bubble up to the window.onerror handler.

Here is a screendump of the appInsights.trackException("String Error") version
image

For the one where I let it bubble to the window.onerror
image

And finally with appInsights.trackexception({ exception: "String Error" }), this is also the same for the try / catch example
image

The screen dumps where taken using F12 dev tools with the breakpoint @

this is after we have created the envelope (the event detail) but before we send it off for batching.

Shorter answer
Please call trackException with an object that conforms to the IExceptionTelemetry interface.

@MSNev MSNev removed the investigation required Further investigation or discussions required label Feb 1, 2023
MSNev added a commit that referenced this issue Feb 1, 2023
@PieterWillemen
Copy link
Author

I just figured out that in 1 place we were in fact using trackException with the wrong type of parameter.

image

Although this will not be the cause of all of our errors.

MSNev added a commit that referenced this issue Feb 1, 2023
MSNev added a commit that referenced this issue Feb 1, 2023
@MSNev MSNev added the fixed - waiting release PR Committed and waiting deployment label Feb 1, 2023
@MSNev
Copy link
Collaborator

MSNev commented Feb 1, 2023

The PR that I've just pushed in (so it will be in the next release) provides a "fallback" cases for cases where someone passes a non-IExceptionTelemetry (doesn't contain an error or exception object) to attempt to convert to provide more information.

This has test coverage and appears to now be handling strings or Error instances directly (as well as null / undefined which should be the only cases for the "not_specified" now)

@PieterWillemen
Copy link
Author

Great!

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants