DavidWells / analytics

Lightweight analytics abstraction layer for tracking page views, custom events, & identifying visitors
https://getanalytics.io
MIT License
2.46k stars 244 forks source link

Clients throwing error: abort is not a function #418

Closed Paso closed 9 months ago

Paso commented 9 months ago

I use analytics.js with some custom plugins that validate and transform the payload. Using Sentry I can see that some of my users throw errors from my plugins when they are trying to call abort. I have been unable to reproduce the error myself but it occurs for several different users in different hardware, browsers and releases.

The error is TypeError: abort is not a function and the stack is as follows:

analytics.js 260:14
@analytics/src/utils/heartbeat 43:11
Array.forEach
@analytics/src/utils/heartbeat 28:29
@analytics/src/utils/heartbeat 78:28

I have tried to understand what the heartbeat does but don't quite understand its use, however I read it as something that is only called on initialization and it appears to me that the call at executes the plugins without an abort argument, but that doesn't make any sense to me so I realize that it probably is just me not understanding what is going on.

One of the plugins that have thrown the error (it occurs in several plugins) follows, what it does should not be relevant as the same error is thrown from different plugins:

const validationPlugin = {
  identifyStart: ({ payload, abort }) => {
    const prev = identifyTraits[payload.userId];
    const traits = { ...identifyTraits[payload.userId], ...payload.traits };
    if (isEqual(prev, traits)) {
      return abort('Identify traits is the same as previous identify');
    }
    identifyTraits[payload.userId] = traits;
    return { ...payload, traits };
  }
}
DavidWells commented 9 months ago

heartbeat ticks every 3 sec and flushes any queued events https://github.com/DavidWells/analytics/blob/dca32cb95cbf8b38d36fcc866d21f9553f29ac24/packages/analytics-core/src/index.js#L982

abort should be a function there. I've tested this locally with another identifyStart plugin. Are you calling abort from anywhere else? Maybe verify it exists before calling it with typeof abort === 'function' check

Also, It looks like you are missing required name key from your plugin. It should be:

const validationPlugin = {
  name: 'my-plugin-name',
  identifyStart: ({ payload, abort }) => {
    const prev = identifyTraits[payload.userId];
    const traits = { ...identifyTraits[payload.userId], ...payload.traits };
    if (isEqual(prev, traits)) {
      return abort('Identify traits is the same as previous identify');
    }
    identifyTraits[payload.userId] = traits;
    return { ...payload, traits };
  }
}
Paso commented 9 months ago

I misread the setInterval as setTimeout, so that explains my confusion about that, but doesn't explain the problem.

The code was just a truncated example so it has a name and other methods, but that is one of the places where the abort function sometimes is missing.

I still don't get how the abort-function i added to the arguments when called from the heartbeat here, because method here is my identifyStart as defined above, right? https://github.com/DavidWells/analytics/blob/dca32cb95cbf8b38d36fcc866d21f9553f29ac24/packages/analytics-core/src/utils/heartbeat.js#L43

I could add a guard against the missing abort, but that seem like a recipe for dropping data so I would rather avoid it.

DavidWells commented 9 months ago

Aha I see.

We probably need a noOp function in heartbeat. Need to test this out

method({
    payload: enrichedPayload,
    config: plugins[currentPlugin].config,
    instance,
    // NoOp on queue drain
    abort: () => {}
})

......

When are you calling analytics.identify in your application? It sounds like you might be running it before all the providers are loaded. This is causing calls to get queued for the heartbeat to flush when it's back. Alternatively, it's from navigator.onLine toggling and the users internet connection going in and out...

You can try to use .ready to avoid the first case

analytics.ready(() => {
   analytics.identify('abc', { name: 'bob' })
})
Paso commented 9 months ago

Why shouldn't the plugins called from heartbeat be abortable?

We use a queue so it is not so easy to tell when the actual analytics.identify is being called.

If it's a problem with providers loading that would explain why the problem is so hard to reproduce, it could be the users network connection or firewall that is the problem. But shouldn't the identify (and others such as track) be queued when called before ready?

DavidWells commented 9 months ago

Why shouldn't the plugins called from heartbeat be abortable?

Yeah probably. The heartbeat was initially setup for simple edge cases though ("Just queue events to fire when network resumes").

The queued items get processed in order here https://github.com/DavidWells/analytics/blob/dca32cb95cbf8b38d36fcc866d21f9553f29ac24/packages/analytics-core/src/utils/heartbeat.js#L43 but the return values aren't checked like the main flow. abort calls just return an object with abort key and this signals downstream actions to halt. You can see queue via Analytics.getState().queue

image

Need to think about how we can get this working for the items shifted off into the queue.

I've reproduced the issue

image
DavidWells commented 9 months ago

Fixed in analytics@0.8.11

Thanks for reporting this edge case