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

bug(common/models): cannot apply transform to null context 🐵 #10136

Closed
sentry-io bot opened this issue Dec 5, 2023 · 4 comments · Fixed by #10185
Closed

bug(common/models): cannot apply transform to null context 🐵 #10136

sentry-io bot opened this issue Dec 5, 2023 · 4 comments · Fixed by #10185

Comments

@sentry-io
Copy link

sentry-io bot commented Dec 5, 2023

Sentry Issue: KEYMAN-WEB-FH

TypeError: undefined is not an object (evaluating 'e.left')
  at None (blob:null/2ef37f8a-965f-4741-9ce2-793daf17264b:5:20146)

@jahorton on the analysis here.

Diving into the lm-worker minified source, I have been able to map the error to the following section:

export function applyTransform(transform: Transform, context: Context): Context {
// First, get the current context
let fullLeftContext = context.left || '';
let lLen = fullLeftContext.kmwLength();

Note line 10 in particular - that's the line that's failing. So... something's attempting to apply a transform to a null context. The natural question is, of course... "what?"


The section of minified code:

extendString();var SENTINEL_CODE_UNIT="\uFDD0";function applyTransform(r,e){
var t,n,i=e.left||"",a=i.kmwLength(),u=a<r.deleteLeft?a:r.deleteLeft,o=i.kmwSubstr(0,a-u) //...

The line-column identifier matches perfectly to i=e|.left - that is, with the caret between the e and the ..

Copy link
Author

sentry-io bot commented Dec 5, 2023

Sentry issue: KEYMAN-WEB-ET

This one appears to line up against a different block, but for the same issue: an unexpected null in place of a context.

My best guess here is this block:

// Using the potential `matchState` + the incoming transform, do the results line up for
// our observed context? If not, skip it.
//
// Necessary to properly handle multitaps, as there are context rewinds that the
// predictive-text engine is not otherwise warned about.
const doublecheckContext = applyTransform(transformDistribution[0].sample, priorMatchState.taggedContext);
if(doublecheckContext.left != context.left) {
continue;
}

The context.left reference lines up for the specified column... but this is new code on the gestures line - see #9855. That said, the event was from a local build - thus easily could have been from the gestures line. It'd also make the most sense - that a change introduced on the gestures line is responsible for an error we haven't really been getting elsewhere.

That said, the version with the associated commit is 17.0.190, with the feature-branch merge around 17.0.200. That said, there may have been some rebasing of the commits involved.

@jahorton
Copy link
Contributor

jahorton commented Dec 5, 2023

BTW, this is behind the sporadic errors that were noticed when trying out #10103 on an Android phone. The version data & timeline matches quite well, and the "environment" for the first linked event is test.

@jahorton jahorton changed the title bug(common/models): cannot apply transform to null context bug(common/models): cannot apply transform to null context 🐵 Dec 5, 2023
@jahorton
Copy link
Contributor

jahorton commented Dec 5, 2023

At any rate, since this is something we don't seem to be seeing in current stable or alpha builds, it's likely feature-gestures related, and that means that the issue likely arose out of #9855 in some manner.

@jahorton
Copy link
Contributor

jahorton commented Dec 7, 2023

Just had a lucky trigger from standalone-Web when testing something else. Here's the log:

common.ts:10 Uncaught TypeError: Cannot read properties of undefined (reading 'left')
    at applyTransform (common.ts:10:33)
    at ContextTracker2.analyzeState (context-tracker.ts:541:38)
    at ModelCompositor2.predict (model-compositor.ts:150:46)
    at Object.handleMessage (index.ts:328:42)
    at LMLayerWorker2.onMessage (index.ts:167:16)

Fortunately, as I was working with full unminified sourcemap access, we got better logs for this event!

My two most-recent keystrokes were BKSP, and I was clearing text. I'd only typed a single "word" to begin with.

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