m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-23 09:52:26 +01:00
Code

Fix metrics error when options are not used (#9985)

Browse Source 
Attempts to send metrics would fail when no `options` were used. This
was because when the options parameter was not set, it was often sent
over our RPC connection as `undefined`, which gets serialized to `null`
when the message is converted to JSON. This `null` parameter didn't
trigger the default parameter set in the metametrics controller, as
default parameters are only used for `undefined`.

Instead the `options` parameter is now treated as fully optional, with
no default value set. The optional chaining operator is used to ensure
it won't blow up if it's not set. A fallback of `{}` was used for the
one destructure case as well.
This commit is contained in:
Mark Stacey 2020-12-03 15:35:11 -03:30 committed by GitHub
parent b1b6d7ae38
commit 703a063ad1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
app/scripts/controllers/metametrics.js
View File

@ -215,7 +215,7 @@ export default class MetaMetricsController {
* event appropriately. * event appropriately.
* @private * @private
* @param {SegmentEventPayload} payload - properties to attach to event * @param {SegmentEventPayload} payload - properties to attach to event
* @param {MetaMetricsEventOptions} options - options for routing and * @param {MetaMetricsEventOptions} [options] - options for routing and
* handling the event * handling the event
* @returns {Promise<void>} * @returns {Promise<void>}
*/ */
@ -225,10 +225,10 @@ export default class MetaMetricsController {
metaMetricsId: metaMetricsIdOverride, metaMetricsId: metaMetricsIdOverride,
matomoEvent, matomoEvent,
flushImmediately, flushImmediately,
} = options } = options || {}
let idType = 'userId' let idType = 'userId'
let idValue = this.state.metaMetricsId let idValue = this.state.metaMetricsId
let excludeMetaMetricsId = options.excludeMetaMetricsId ?? false let excludeMetaMetricsId = options?.excludeMetaMetricsId ?? false
// This is carried over from the old implementation, and will likely need // This is carried over from the old implementation, and will likely need
// to be updated to work with the new tracking plan. I think we should use // to be updated to work with the new tracking plan. I think we should use
// a config setting for this instead of trying to match the event name // a config setting for this instead of trying to match the event name
@ -282,15 +282,15 @@ export default class MetaMetricsController {
/** /**
* track a page view with Segment * track a page view with Segment
* @param {MetaMetricsPagePayload} payload - details of the page viewed * @param {MetaMetricsPagePayload} payload - details of the page viewed
* @param {MetaMetricsPageOptions} options - options for handling the page * @param {MetaMetricsPageOptions} [options] - options for handling the page
* view * view
*/ */
trackPage({ name, params, environmentType, page, referrer }, options = {}) { trackPage({ name, params, environmentType, page, referrer }, options) {
if (this.state.participateInMetaMetrics === false) { if (this.state.participateInMetaMetrics === false) {
return return
} }
if (this.state.participateInMetaMetrics === null && !options.isOptInPath) { if (this.state.participateInMetaMetrics === null && !options?.isOptInPath) {
return return
} }
const { metaMetricsId } = this.state const { metaMetricsId } = this.state
@ -316,16 +316,16 @@ export default class MetaMetricsController {
* with sensitiveProperties into two events, tracking the sensitiveProperties * with sensitiveProperties into two events, tracking the sensitiveProperties
* with the anonymousId only. * with the anonymousId only.
* @param {MetaMetricsEventPayload} payload - details of the event * @param {MetaMetricsEventPayload} payload - details of the event
* @param {MetaMetricsEventOptions} options - options for handling/routing the event * @param {MetaMetricsEventOptions} [options] - options for handling/routing the event
* @returns {Promise<void>} * @returns {Promise<void>}
*/ */
async trackEvent(payload, options = {}) { async trackEvent(payload, options) {
// event and category are required fields for all payloads // event and category are required fields for all payloads
if (!payload.event || !payload.category) { if (!payload.event || !payload.category) {
throw new Error('Must specify event and category.') throw new Error('Must specify event and category.')
} }
if (!this.state.participateInMetaMetrics && !options.isOptIn) { if (!this.state.participateInMetaMetrics && !options?.isOptIn) {
return return
} }
@ -337,7 +337,7 @@ export default class MetaMetricsController {
// sensitiveProperties will only be tracked using the anonymousId property and generic id // sensitiveProperties will only be tracked using the anonymousId property and generic id
// If the event options already specify to exclude the metaMetricsId we throw an error as // If the event options already specify to exclude the metaMetricsId we throw an error as
// a signal to the developer that the event was implemented incorrectly // a signal to the developer that the event was implemented incorrectly
if (options.excludeMetaMetricsId === true) { if (options?.excludeMetaMetricsId === true) {
throw new Error( throw new Error(
'sensitiveProperties was specified in an event payload that also set the excludeMetaMetricsId flag', 'sensitiveProperties was specified in an event payload that also set the excludeMetaMetricsId flag',
) )