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

TypeErrors in diagnosticEvents #65

Closed
farrelje opened this issue Apr 12, 2022 · 5 comments
Closed

TypeErrors in diagnosticEvents #65

farrelje opened this issue Apr 12, 2022 · 5 comments

Comments

@farrelje
Copy link

Describe the bug
In our frontend logs, there seem to be a large number of errors of the variety TypeError: o.push is not a function at Object.recordStreamInit, which seems to be originating in js-sdk-common/src/diagnosticEvents.js](https://github.com/launchdarkly/js-sdk-common/blob/bee90f5d5f4acfb64def652c57e9e5d45e05439b/src/diagnosticEvents.js#L53-59).

Describe the solution you'd like
A guard in place to prevent pushing data into non-array elements to reduce this sort of error possibility.

@eli-darkly
Copy link
Contributor

Thanks for reporting this; I'm pretty sure we've never seen such an issue before. It's very odd and I'm having trouble seeing how streamInits could ever be initialized to anything other than an array; the only place where it's set to anything other than a literal [] is on line 45, but there the parameter is coming from JSON data that was originally serialized from those same values.

@kinyoklion
Copy link
Member

Hello @farrelje,

Would you be able to tell us what tools and versions you are using in your build process (webpack/babel/etc.)? If possible could we get a segment of the bundled code around where this error is being emitted? Additionally does the origin of these errors seem to be a specific browser?

Thank you,
Ryan

@farrelje
Copy link
Author

Hi @kinyoklion, I've had a good look through our logs and it doesn't seem to be browser specific at all. As this library is used as part of another LD JS SDK, I've discussed the issue with the internal library team implementing the LD code, and it seems to simply push the data and work as intended.

As for tools, it's a bit of a complicated setup (we are also in the middle of upgrading our tools, so it will be worth monitoring when any differences when complete), but essentially we are using:

  • webpack - 5.64.1,
  • babel - 7.16

Here's a snippet from bundled code where the errors seem to be occurring:

return (
  a(e),
  {
    getProps: function () {
      return {
        dataSinceDate: t,
        droppedEvents: r,
        eventsInLastBatch: n,
        streamInits: o,
      };
    },
    setProps: function (e) {
      (t = e.dataSinceDate),
        (r = e.droppedEvents || 0),
        (n = e.eventsInLastBatch || 0),
        (o = e.streamInits || []);
    },
    incrementDroppedEvents: function () {
      r++;
    },
    setEventsInLastBatch: function (e) {
      n = e;
    },
    recordStreamInit: function (e, t, r) {
      var n = { timestamp: e, failed: t, durationMillis: r };
      o.push(n);
    },
    reset: a,
  }
);

Hopefully this is helpful in some way!

Regards,
Farrel

@kinyoklion
Copy link
Member

Thank you @farrelje

I will look through the code and see if I can tell anything. The SDK code itself looks correct here, so I am trying to see if anything in the supporting ecosystem is causing a problem.

The only real way that the SDK code could produce the error would be if the contents of local storage were modified. Such that streamInits still exists, but would deserialize to a different type. This probably isn't what is happening, so I wanted to check the minifed code to see if there were any problems there. (An example of where the minifier could cause a problem mishoo/UglifyJS#5155)

I am not seeing anything in this snippet, but I also do not see the creation of o. I am curious if there is any other usage of that variable that may conflict or cause the type to be coerced to an incorrect type. (Like + with a string will produce a string instead of a usage error.)

I would be hesitant to add a guard without knowing the underlying cause. It is possible it will not help, but also possible that it will just introduce a more subtle malfunction.

Thank you,
Ryan

LaunchDarklyReleaseBot pushed a commit that referenced this issue Jun 6, 2022
…s-documentation

Add doc for AllFlags to refer to sendEventsOnlyForVariation
@farrelje
Copy link
Author

Closing as has been resolved - seems to have been caused by a (difficult to find) incorrect polyfill. Thanks for your patience with this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants