-
Notifications
You must be signed in to change notification settings - Fork 235
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
include response error data #1161
Changes from 1 commit
9339b31
92f4433
7a06819
ffdf8d1
78f33fd
fb6cbb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,8 @@ export class AjaxMonitor extends BaseTelemetryPlugin implements IDependenciesPlu | |
appId: undefined, | ||
enableCorsCorrelation: false, | ||
enableRequestHeaderTracking: false, | ||
enableResponseHeaderTracking: false | ||
enableResponseHeaderTracking: false, | ||
includeResponseErrorData: false | ||
} | ||
return config; | ||
} | ||
|
@@ -66,7 +67,8 @@ export class AjaxMonitor extends BaseTelemetryPlugin implements IDependenciesPlu | |
enableCorsCorrelation: undefined, | ||
correlationHeaderDomains: undefined, | ||
enableRequestHeaderTracking: undefined, | ||
enableResponseHeaderTracking: undefined | ||
enableResponseHeaderTracking: undefined, | ||
includeResponseErrorData: undefined | ||
} | ||
} | ||
|
||
|
@@ -557,6 +559,11 @@ export class AjaxMonitor extends BaseTelemetryPlugin implements IDependenciesPlu | |
} | ||
} | ||
|
||
if (_self._config.includeResponseErrorData && xhr.status !== 200) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do we want to do this for all !=200 status codes or should we just check >=400 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 we should only do this for status >= 400 As any response from 200 -> 299 is considered to be a success response, a common response is 204 == No Content. |
||
dependency.properties = dependency.properties || {}; | ||
dependency.properties.responseError = xhr.statusText + " - " + xhr.responseText; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: rename to |
||
} | ||
|
||
_self.trackDependencyDataInternal(dependency); | ||
|
||
xhr.ajaxData = null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -294,6 +294,14 @@ export interface IConfig { | |
*/ | ||
enableResponseHeaderTracking?: boolean; | ||
|
||
/** | ||
* @description An optional value that will track Resonse Error data through trackDependency function. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small typo in description |
||
* @type {boolean} | ||
* @memberof IConfig | ||
* @defaultValue false | ||
*/ | ||
includeResponseErrorData?: boolean; | ||
|
||
/** | ||
* @description Default false. when tab is closed, the SDK will send all remaining telemetry using the [Beacon API](https://www.w3.org/TR/beacon) | ||
* @type {boolean} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be renamed to
disableAjaxErrorStatusText
since it seems like something we would want to enable by default.