keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
398 stars 112 forks source link

bug(web): `undefined` TypeError being raised on Android 5.1.1 devices #11502

Closed sentry-io[bot] closed 5 months ago

sentry-io[bot] commented 5 months ago

Sentry Issue: KEYMAN-WEB-K4

TypeError: undefined is not a function
  at None (blob:null/2f613f94-4628-45cd-b143-cd67d57566d3:5:69)
jahorton commented 5 months ago

Note the blob URL - this is being raised from the predictive-text worker.

jahorton commented 5 months ago

What I'm seeing here doesn't seem to make much sense.

The line for the error report, based on the minified ES5 version of the worker's code:

*/Array.prototype.findIndex||Object.defineProperty(Array.prototype,"findIndex", /* ... */

Line 5, column 69: "|findIndex" - that is, after the opening quotation mark of the string "findIndex". But... that shouldn't be capable of generating this error, right?


Also of interest: so far, this is exclusively from Chrome for Android WebView 39.0 and vivo Y31A devices. It does not reproduce with a local, Android-Studio-emulator-based Android 5.0 device.

jahorton commented 5 months ago

There are indications that there is an emulation target that can trigger it: there are now a few entries for "Android SDK built for x86". No other devices have reported an issue.

jahorton commented 5 months ago

Setting up a Pixel 2 XL API 22 was sufficient to get a local reproduction. Sadly, attempts to "inspect device" through Chrome are currently instantly "detached", so I may need to figure out what that's about.

It does, at least, give us a line within the unminified version of app/webview:

blob:null/e4ffe3c5-df90-4811-a92c-33bf39e648b0 at line 5258:24

So that's something I can chase, even without inspection.

jahorton commented 5 months ago

So far, I can tell that it occurs while generating predictions - in particular, from the predictFromCorrections method of ModelCompositor.

Reference, after adding custom errors as a substitute for console logging b/c it's worker code:

https://keyman.sentry.io/issues/5359770502/events/f009a872785b4ba68a749414a9530ba4/

It's hard to conclusively state that this is the only error that would be thrown, but it's certainly the first.

Edit: it's coming from lexical-model code. It's probably from the TrieModel template somehow.

https://keyman.sentry.io/issues/5359770502/events/a7d0e9154fce488b9f42b70d364e18c1/

An extra line would have been included in the error message had line 47 below completed successfully:

https://github.com/keymanapp/keyman/blob/614f957de583e68aaad9193c397bcc88ff065c09/common/web/lm-worker/src/main/model-compositor.ts#L46-L49

Note - it often does complete successfully; the method that code block is in successfully returns multiple times before the error is first thrown.

jahorton commented 5 months ago

I've now isolated the error to the following call:

https://github.com/keymanapp/keyman/blob/614f957de583e68aaad9193c397bcc88ff065c09/common/models/templates/src/trie-model.ts#L257

And even further down that that, to here:

https://github.com/keymanapp/keyman/blob/614f957de583e68aaad9193c397bcc88ff065c09/common/models/templates/src/tokenization.ts#L32

Drilling ever deeper, I've now isolated the error to this call:

https://github.com/keymanapp/keyman/blob/614f957de583e68aaad9193c397bcc88ff065c09/common/models/wordbreakers/src/default/index.ts#L52

And found it! There was a lingering use-case of Array.from within the wordbreaker code that wasn't phased out when we fixed all the other Array.from cases.

https://github.com/keymanapp/keyman/blob/614f957de583e68aaad9193c397bcc88ff065c09/common/models/wordbreakers/src/default/index.ts#L277

jahorton commented 5 months ago

With no Array.from polyfill in place, this will also trigger errors:

https://github.com/keymanapp/keyman/blob/614f957de583e68aaad9193c397bcc88ff065c09/developer/src/kmc-model/src/model-defaults.ts#L58

This code is baked into a number of currently in-distribution lexical-models. As a result, it's clear that removing the Array.from polyfill was removed too soon; we still need it. Fun detail though - the polyfill we were previously using did not account for surrogate pairs like the real, modern Array.from does.