bug(developer): tran failed unexpectedly

[sentry[bot]]

Sentry Issue: KEYMAN-DEVELOPER-1QY

Error: The compiler for 'tran' failed unexpectedly.
  File "/C:/Program%20Files%20(x86)/Keyman/Keyman%20Developer/kmc/kmc.mjs", line 347989, in KeymanSentry.reportException
  File "/C:/Program%20Files%20(x86)/Keyman/Keyman%20Developer/kmc/kmc.mjs", line 361088, in NodeCompilerCallbacks.reportMessage
  File "/C:/Program%20Files%20(x86)/Keyman/Keyman%20Developer/kmc/kmc.mjs", line 194686, in CompilerFileCallbacks.reportMessage
  File "/C:/Program%20Files%20(x86)/Keyman/Keyman%20Developer/kmc/kmc.mjs", line 359793, in LdmlKeyboardCompiler.compile
  File "/C:/Program%20Files%20(x86)/Keyman/Keyman%20Developer/kmc/kmc.mjs", line 359675, in LdmlKeyboardCompiler.run
...
(4 additional frame(s) were not displayed)

[srl295]

The caller has

        // This should not happen -- validate() should have told us
        // if something is going to fail to compile

[mcdurdin]

Hmm, we don't have sourcemap working there. Another day. Another issue.

[srl295]

#10937 ~~10397~~ may be somewhat of the root cause.

The proximate cause is the "fatal exception" shouldn't happen if we've already given an error message for that compiler.

This is somewhat related to the other issue (#?) about line numbers, in that we may need to do a little bit better with tracking error messages. That is, if we've already given an error, don't give an error that we gave an error.

[mcdurdin]

For sure fatal exceptions are indicative of a bug in the compiler. Line numbers should be fixed for alpha with the next merge from beta to alpha. They are already working in beta.

[mcdurdin]

#10937 ~~10397~~ may be somewhat of the root cause.

How? Oh I see, editing the issue number now.

Note: deleted the ref to the other issue to avoid future confusion

[srl295]

This is the issue:

$ node developer/src/kmc build developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml 
fail-bad-tran-2.xml - info KM05002: Building developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml
fail-bad-tran-2.xml - error KM00030: Invalid escape "\u0127", use "\u{127}" instead.
fail-bad-tran-2.xml - fatal KM0000F: Unexpected exception: The compiler for 'tran' failed unexpectedly.

Call stack:
Error
    at CompilerMessageSpecWithException (file:///C:/projects/keyman/keyman/common/web/types/build/src/util/compiler-interfaces.js:351:152)
    at CompilerMessages.Fatal_SectionCompilerFailed (file:///C:/projects/keyman/keyman/developer/src/kmc-ldml/build/src/compiler/messages.js:39:49)
    at LdmlKeyboardCompiler.compile (file:///C:/projects/keyman/keyman/developer/src/kmc-ldml/build/src/compiler/compiler.js:290:67)
    at async LdmlKeyboardCompiler.run (file:///C:/projects/keyman/keyman/developer/src/kmc-ldml/build/src/compiler/compiler.js:82:19)
    at async BuildLdmlKeyboard.runCompiler (file:///C:/projects/keyman/keyman/developer/src/kmc/build/src/commands/buildClasses/BuildActivity.js:9:24)
    at async BuildLdmlKeyboard.build (file:///C:/projects/keyman/keyman/developer/src/kmc/build/src/commands/buildClasses/BuildLdmlKeyboard.js:16:16)
    at async build (file:///C:/projects/keyman/keyman/developer/src/kmc/build/src/commands/build.js:138:22)
    at async Command.buildFile (file:///C:/projects/keyman/keyman/developer/src/kmc/build/src/commands/build.js:92:14)
    at async Command.parseAsync (C:\projects\keyman\keyman\node_modules\commander\lib\command.js:935:5)
    at async run (file:///C:/projects/keyman/keyman/developer/src/kmc/build/src/kmc.js:54:5)

    This error has been automatically reported to the Keyman team.
      Identifier:  c6ba035695e0452c91ec8c111a918291
      Application: Keyman Developer
      Reported at: https://sentry.io/organizations/keyman/projects/keyman-developer/events/c6ba035695e0452c91ec8c111a918291/
    fail-bad-tran-2.xml - info KM05007: developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-tran-2.xml failed to build.

• an error (the bad escaped char) was reported - but very deep in the analysis
• the 'fatal' is because the tran compiler returned null

I propose that we have some way to detect that we've already emitted an error. The abstract CompilerCallbacks doesns't have such an interface.

Changing how the callbacks works is where the relation to line numbers comes in, because it's an architectural change to callbacks.

[mcdurdin]

I propose that we have some way to detect that we've already emitted an error. The abstract CompilerCallbacks doesns't have such an interface.

Yeah I am not sure. That means the generating a compiler error message is a side-effect that is tracked externally, reporting a signal back to the module itself. The module should be self-aware enough to know it has failed. Returning null from the tran compiler should probably be that signal, rather than counting compiler error messages -- is there any reason we'd ever expect a null from the tran or any other compiler for anything else other than reported failure? (Noting that a real failure would normally be a throw anyway)

[srl295]

(from slack)

So: I'm wondering if there would be some value in an 'error cursor/excursion' object. That is, it's a set of errors/warnings that's about to be added to the callback list, but it can be kind of deduplicated. It might be as simple as a CompilerEvent[] array. But the idea is that a process such as the keys or tran compiler could pass around a list of errors that this subcomponent is about to pass back into the message stream.. to avoid duplicates, and also to avoid, say, errors about a broken transform, that depends on a variable that we've already given an error about.

[srl295]

I propose that we have some way to detect that we've already emitted an error. The abstract CompilerCallbacks doesns't have such an interface.

Yeah I am not sure. That means the generating a compiler error message is a side-effect that is tracked externally, reporting a signal back to the module itself. The module should be self-aware enough to know it has failed. Returning null from the tran compiler should probably be that signal, rather than counting compiler error messages -- is there any reason we'd ever expect a null from the tran or any other compiler for anything else other than reported failure? (Noting that a real failure would normally be a throw anyway)

Perhaps the internal error isn't needed then.

[mcdurdin]

Perhaps the internal error isn't needed then.

It seems that way to me. The internal error would be if tran didn't report a compiler error but still returned null -- and that could be a unit test, although it's a negative test so pretty hard to write