search

oguzgelal / juken

Review App for WaniKani
https://juken.netlify.app
MIT License
28 stars 6 forks source link

Too many reviews (?) cause ”maximum call stack size reached” #65

Closed asutekku closed 3 years ago

asutekku commented 4 years ago

That’s at least my assumption without looking at the code, as at the moment i have 1476 reviews waiting.

Error comes up when the app fetches data during startup.

Related error screen is attached.

4E9F060B-6767-4B45-9CCB-8BDE24E072F1

joyalicegu commented 4 years ago

Sounds like a problem with adjustQueue (which is recursive).

joyalicegu commented 4 years ago

Does this happen on the web version at juken.io also?

oguzgelal commented 4 years ago

@joyalicegu Thanks a lot for the PR! I merged it in. I do agree that avoiding recursion is a good idea to avoid potential call stack errors for low memory devices, but I'm sure that it would fix this particular issue.

I increased the number of reviews available manually from freeAssignments.js mock file and tested it in demo mode, and it seemed to work with around 5k reviews. I suspect it's somehow related to requests, and the parsing of the requests.

@asutekku Did the same issue happen in the browser ? Also, could I ask what your device is ? (note to self, device details should also be on the error screen 🙂)

oguzgelal commented 4 years ago

Whoops didn't mean to close the ticket

asutekku commented 4 years ago

Does not happen on web version on iOS 14, does happen on app version on iOS 14. Prior to this, worked perfectly on iOS 14, started to misbehave only after certain amount of reviews in queue was exceeded.

asutekku commented 4 years ago

The fix did not fix the issue. I tried with a new api key and same thing keeps happening.

drydenb commented 4 years ago

I downloaded Juken today and am also experiencing this issue (or what appears to be the same issue) on an iPhone 11 Pro Max on iOS 14.0.1.

Similar to the original report, I also have a large amount of reviews to do (~1500).

I am attaching my screenshot as well in case it is useful. Scrolling down to the bottom of the crash log looks to be the same as the issue here.

screenshot

Excited to try this app out once the issue is resolved. Thanks!

joyalicegu commented 4 years ago

The iOS app hasn't been updated since July 20th. The fix was applied to the web version in August.

Edit: Or rather, a change that may or may not fix this particular issue was applied to the web version in August.

asutekku commented 4 years ago

@oguzgelal Any ETA on updating the app on AppStore?

oguzgelal commented 4 years ago

@asutekku @joyalicegu My apologies for the lack of support on this. I wasn't eager to push up a new version because I have doubts that the fix solves the issue. The issue wasn't reproducible by @joyalicegu and me, replacing the recursive algorithm with a while loop is a good idea in general, but for this issue in particular it's kind of a shot in the dark.

The recursive algorithm was linear, and the reported number of reviews that causes the stack overflow issue is around 1000 - 1500. The issue was reported on newer iPhone models and iOS versions. I believe the problem is related to the way adjustSubjectPairDistances is called rather than its implementation. Or perhaps something iOS related as it is the only platform this is reported.

Also admittedly it's a rather overwhelming period in my life and I haven't had the chance to work on Juken much lately. That's why I haven't been able to investigate this any further. Though I reckon @joyalicegu 's fix is still promising and worth a shot, @asutekku @drydenb if I were to roll out the current version on Testflight, would you guys be interested in test it there ? If so, could send your email addresses to o.gelal77@gmail.com so I could invite you guys as a beta tester ?

Thanks!

joyalicegu commented 4 years ago

I'd be interested in beta testing, too. As my overdue reviews increase, I'll probably hit this issue soon. You already have my email address.

oguzgelal commented 4 years ago

Thanks @joyalicegu ! I just submitted a new build and added you as a tester. The build is now processing, I'll start the testing right after its done.

oguzgelal commented 4 years ago

For anyone who'd like to test this out, here is the public link: https://testflight.apple.com/join/jwLDuDhY

oguzgelal commented 3 years ago

It looks like this fixes the issue https://github.com/oguzgelal/juken/pull/76 fixes this issue, thank you @joyalicegu ! I will close this ticket for now and submit a new version to the app store. @asutekku @drydenb please feel free to reopen if the new version won't resolve it for you.