e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

πŸ› Tracking errors on the new dashboard tab #986

Closed JGreenlee closed 7 months ago

JGreenlee commented 1 year ago

Reproducible on prod JS with my personal nrel-commute OPCode on both phone + devapp. Not reproducible on dev JS.

[Error] TypeError: o is not an Object. (evaluating 's in o')
(anonymous function) β€” metrics-factory.js:44
(anonymous function) β€” CarbonFootprintCard.tsx:65
Ka β€” react-dom.production.min.js:179:119
YQ β€” CarbonFootprintCard.tsx:29
wa β€” react-dom.production.min.js:167:139
xs β€” react-dom.production.min.js:197:261
Ml β€” react-dom.production.min.js:280:392
yl β€” react-dom.production.min.js:280:323
gl β€” react-dom.production.min.js:280:180
sl β€” react-dom.production.min.js:271:91
al β€” react-dom.production.min.js:268:432
al
z β€” scheduler.production.min.js:13:205
q β€” scheduler.production.min.js:14:128

    (anonymous function) (index.html:92)
    (anonymous function) (cordova.js:1413)
    fs (bundle.js:2:2229277)
    (anonymous function) (bundle.js:2:2229502)
    Ni (bundle.js:2:2211107)
    Oc (bundle.js:2:2258732)
    Mc (bundle.js:2:2258167)
    vc (bundle.js:2:2257705)
    (anonymous function) (bundle.js:2:2269482)
    zl (bundle.js:2:2269991)
    al (bundle.js:2:2262493)
    al
    z (bundle.js:2:2726060)
    q (bundle.js:2:2726592)
JGreenlee commented 1 year ago

Also reproducible on prod JS, this time on a usaid-laos opcode with no trips. Not producible on dev JS or any other codes I tried.

[Error] Error: "bar" is not a registered controller.
value β€” core.registry.js:178
value β€” core.controller.js:455
value β€” core.controller.js:508
(anonymous function) β€” chart.tsx:85
ic β€” react-dom.production.min.js:244:332
Cl β€” react-dom.production.min.js:286:114
(anonymous function) β€” react-dom.production.min.js:282:391
O β€” scheduler.production.min.js:13:205
q β€” scheduler.production.min.js:14:128
    (anonymous function) (cordova.js:1413)
    fs (bundle.js:2:2221533)
    (anonymous function) (bundle.js:2:2221758)
    Ni (bundle.js:2:2203363)
    zc (bundle.js:2:2250988)
    Mc (bundle.js:2:2250423)
    vc (bundle.js:2:2249961)
    (anonymous function) (bundle.js:2:2261738)
    Ol (bundle.js:2:2262247)
    ul (bundle.js:2:2256008)
    ul
    Yr (bundle.js:2:2196770)
    Cl (bundle.js:2:2263584)
    (anonymous function) (bundle.js:2:2260046)
    O (bundle.js:2:2718316)
    q (bundle.js:2:2718848)
[Error] Error: "bar" is not a registered controller.
    Yr (bundle.js:2:2196861)
    Cl (bundle.js:2:2263584)
    (anonymous function) (bundle.js:2:2260046)
    O (bundle.js:2:2718316)
    q (bundle.js:2:2718848)
JGreenlee commented 1 year ago

For the [Error] Error: "bar" is not a registered controller error, I can't find any reason this would occur except that BarElement is not being registered with ChartJS.

ChartJS is tree-shakable and it accomplishes this by having each element be individually registered. So you only import the elements you actually need. We import the ones we need at https://github.com/e-mission/e-mission-phone/blob/87b3456cce53ebdf3bebfaf88332ef8270adda16/www/js/components/Chart.tsx#L10.

I added a debugger breakpoint on the above line to make sure it was actually being reached. It is, but we get an error anyway. What's weird is that it works fine on development builds. It is only when Webpack is in production mode that we see the error. So this is some kind of incompatibility between the ChartJS library and Webpack. Probably not something we can fix ourselves.

JGreenlee commented 1 year ago

Plenty of people online have struggled with this error because they didn't properly register the required Chart elements.

https://stackoverflow.com/questions/67060070/chart-js-core-js6162-error-error-line-is-not-a-registered-controller/67143648#67143648 The accepted answer on this stackoverflow thread says we could just import Chart from 'chart.js/auto and it would automatically register everything.

We would miss out on tree-shaking but it might be our only option.

JGreenlee commented 1 year ago

Well import Chart from 'chart.js/auto' didn't work - it gave exactly the same result.

However, I tried the other option from the above stackoverflow thread, which was to import all registerables at once and then register them all at once.

That worked. So I think that's what we're going to go with right now. And then I'll also file an issue on the ChartJS repo, hopefully to get resolved eventually.

JGreenlee commented 1 year ago

Unfortunately, this does mean that in the meantime we have to carry the entire bundle size of ChartJS (which I'm frankly not sure how big it is)

JGreenlee commented 1 year ago

I haven't been able to reproduce the TypeError: o is not an Object today. There is a chance it was related to the other issue and was fixed in the process. There's also a chance that it comes up more sporadically and just isn't consistently reproducible.

JGreenlee commented 1 year ago

I think we should just get #1039 merged so we can continue testing on staging. And we'll keep an eye out for a TypeError: o is not an Object in case it comes back.

shankari commented 1 year ago

Likely an incompatibility between ChartJS and Webpack.

Gah! That's pretty bad. However, if we make the app more visual, maybe we will end up using more of the charts anyway. Please link the ChartJS issue here so we can find it and maybe check in on it periodically.

Abby-Wheelis commented 10 months ago

@JGreenlee @shankari I'm not sure if this relates to earlier errors, but I am experiencing an error with the dashboard with my nrel-commute opcode. Whenever I navigate to the dashboard tab, it loads briefly, flashing the first card, and then goes to WSOD. I've been able to reproduce killing and reopening on my phone, and can now reproduce in the emulator.

https://github.com/e-mission/e-mission-docs/assets/54848919/dcfb4b89-a12b-4a7e-8157-f26ef40cad4b

The error I see in the debugger is TypeError: undefined is not an object (evaluating 'chartDatasets[0].data[i].x') the longer error message afterwards makes it seem as if the error is coming from the chart in the weekly active minutes card.

To add, this is not happening on any of my test phones, so maybe it is specific to the opcode?

Abby-Wheelis commented 10 months ago

With that same opcode on both my personal phone and in the emulator this morning the dashboard is working fine, no WSOD.

shankari commented 10 months ago

I wonder if #1025 is a dup of this. If you see the video there, the label screen flashes for a second before going to WSOD.

JGreenlee commented 10 months ago

I wonder if #1025 is a dup of this. If you see the video there, the label screen flashes for a second before going to WSOD.

I don't think it is the same issue, but I did also include some logs in https://github.com/e-mission/e-mission-phone/pull/1100 that might help us debug this one.

JGreenlee commented 10 months ago
image

The issue is in commHelper: https://github.com/e-mission/e-mission-phone/blob/service_rewrite_2023/www/js/commHelper.ts#L132-L137 It is looking for $rootScope, which doesn't exist anymore (probably since onboarding changes).

So this seems like a sneaky regression caused by the interaction of 2 PRs that were merged independently

shankari commented 10 months ago

why is it intermittent, though? It seems like @Abby-Wheelis ran into it one day but not the next.

JGreenlee commented 10 months ago

This is a different issue altogether. I think the dashboard has gotten pretty messed up with all the rewrites and it is going to need a batch of bugfixes before the next release

Abby-Wheelis commented 10 months ago

WSOD on dashboard again for my NREL-commute opcode, with the same errors

The error I see in the debugger is TypeError: undefined is not an object (evaluating 'chartDatasets[0].data[i].x')

This is happening on my metrics-services-rewrite, which already has the fix for $rootScope, with the new log statements, I think I might have id'd the issue. I saw the following log statement:

Screenshot 2023-11-13 at 11 33 34 AM

This indicates that it is looking for chartDatasets[0].data[i].x, which does not exist. We do have multiple chartDatasets but their data entries don't always have multiple values. I switched line line to chartDatasets[i].data[0].x which resolved the error, but I don't understand what i means here enough to be confident in that as a principled fix.

It seems to be accurate, and only needs to be done for the vertical axis labeling (could the issue be that vertical axis is different than horizontal axis?). I have tried lots of combinations of weeks on my opcode, and it works for weeks that have one mode or two and seems to be accurately labeled, so experimentally it's a fix.

It would explain how this is intermittent, at least for me, as what's showing in my "active minutes" has varied a lot. Right now, I biked a lot two weeks ago, but walked a lot this past week, and time is lined up such that I do not have a mix of modes, so we have "walk" and "bike" that each only have one data point. However, two days ago, there would have been a mix because my bike ride and my walks fell within the same week, so the data structure is constantly changing, at least a little bit.

Abby-Wheelis commented 8 months ago

I just logged in to my nrel-commute opcode where I was having this issue on my test phone. I went back and picked dates from this time period where I had active minutes: just walk both weeks, just bike one week and just walk the next, and time where bike and walk occurred in the same week. The dashboard worked in all three of these scenarios, so I can confirm that the changes I made here seem to have successfully resolved that dashboard error.