Skip to content

fix: harden startup and keybinding paths against errors seen in alpha.38 - #104

Merged
dubadub merged 2 commits into
mainfrom
fix/sentry-alpha38-startup-and-keybinding-errors
Aug 20, 2026
Merged

fix: harden startup and keybinding paths against errors seen in alpha.38#104
dubadub merged 2 commits into
mainfrom
fix/sentry-alpha38-startup-and-keybinding-errors

Conversation

@dubadub

@dubadub dubadub commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Fixes the five real issues Sentry reported against 0.1.0-alpha.38 in the first hours of telemetry from real users. All were unhandled errors, none crashed the app.

Issue Error Events
EDITOR-2 TypeError: Object has been destroyed 3
EDITOR-3 A resource provider for 'user-storage:/user/toolbar.json' is not registered 2
EDITOR-4 / EDITOR-6 Cannot read properties of undefined (reading 'keyCode') 2
EDITOR-5 Cannot get key code from the keyboard event 1

Splash screen — EDITOR-2

The maxDuration timer outlives the windows it closes. Quitting during startup destroys both windows, and splashScreenWindow.close() then throws. Guarded with isDestroyed() — including mainWindow.isVisible() on the same path, which fails the same way; the reported frame was just whichever ran first.

Monaco keybinding dispatch — EDITOR-4 / EDITOR-6

KeyCode.createKeyCode deliberately leaves key unset for modifier-only chords, so the keyCode.key! assertion in toKeybinding is not sound. It now returns undefined and getDispatchChords emits null, which its (string | null)[] signature already allows — the same handling getSingleModifierDispatchPart directly above it already applies to this case.

Keybinding registry — EDITOR-5

Some layouts and IMEs deliver events with no usable code, keyCode or keyIdentifier. KeyCode.createKeyCode throws on those, and KebindingRegistry.run is called straight from a keydown listener, so the error escapes unhandled. No keybinding can match such an event, so it is dropped. Both EDITOR-5 and EDITOR-4/6 came from one user in France on Windows 10 within the same second — likely one keypress hitting two listeners.

Toolbar storage — EDITOR-3

@postConstruct init() left doInit() floating, so a failure to resolve the config surfaced as an unhandled rejection; it is now handled (the toolbar already falls back to its defaults).

DefaultResourceProvider.get also swallowed every resolver error before reporting that nothing handled the URI — which is why the Sentry report says nothing about the underlying cause. They are now attached as the error's cause.

This is the part I am least confident about. The catch makes the symptom benign and the next occurrence diagnosable, but it does not fix why the user-storage: provider was unavailable at that moment. That needs the cause data this PR adds.

Verification

  • lerna run compile passes for @theia/core, @theia/monaco, @theia/toolbar; eslint clean on all touched files.
  • Both root causes confirmed empirically against the compiled code — a modifier-only event yields key === undefined, and an event with no usable code/keyCode throws out of createKeyCode.
  • Added a regression test in keybinding.spec.ts for the unmappable-event case.

⚠️ The test suite was not run. This machine has Node 20.11.1 and the repo requires ≥ 22, so mocha refuses to start. The new test is type-checked and compiles but has never executed — please confirm CI is green before merging.

The splash-screen and toolbar fixes are defensive guards validated by inspection; neither race was reproduced locally.

Fixes the five real issues reported by Sentry against 0.1.0-alpha.38 in the
first hours of telemetry from real users.

Splash screen (EDITOR-2, `TypeError: Object has been destroyed`)
The `maxDuration` timer outlives the windows it closes. Quitting during
startup destroys both windows, and `splashScreenWindow.close()` — or
`mainWindow.isVisible()` on the same path — then throws. Guard both with
`isDestroyed()`.

Monaco keybinding dispatch (EDITOR-4, EDITOR-6, `Cannot read properties of
undefined (reading 'keyCode')`)
`KeyCode.createKeyCode` deliberately leaves `key` unset for modifier-only
chords, so the `keyCode.key!` assertion in `toKeybinding` is not sound.
Return `undefined` there and let `getDispatchChords` emit `null`, which its
signature already allows — matching how `getSingleModifierDispatchPart`
right above it already handles the same case.

Keybinding registry (EDITOR-5, `Cannot get key code from the keyboard
event`)
Some layouts and IMEs deliver events with no usable `code`, `keyCode` or
`keyIdentifier`. `KeyCode.createKeyCode` throws on those, and
`KeybindingRegistry.run` is called straight from a `keydown` listener, so
the error escapes unhandled. No keybinding can match such an event; drop it.

Toolbar storage (EDITOR-3, `A resource provider for
'user-storage:/user/toolbar.json' is not registered`)
`@postConstruct init()` left `doInit()` floating, so a failure to resolve
the config surfaced as an unhandled rejection. Handle it — the toolbar
already falls back to its defaults. `DefaultResourceProvider.get` also
swallowed every resolver error before reporting that nothing handled the
URI, which is why the report says nothing about the underlying cause;
attach them as the error's `cause` so the next occurrence is diagnosable.
The spec's module-level JSDOM is torn down before these cases run, so tests
that construct DOM objects re-enable it locally. The new test did not, and
failed with `ReferenceError: KeyboardEvent is not defined`.
@dubadub
dubadub merged commit 4f2a262 into main Aug 20, 2026
2 checks passed
@dubadub
dubadub deleted the fix/sentry-alpha38-startup-and-keybinding-errors branch August 20, 2026 07:53
dubadub added a commit that referenced this pull request Aug 20, 2026
PR #104's first run was killed at 6h0m14s, GitHub's default job limit. It had
hung in `apt-get update`: all four azure.archive.ubuntu.com entries were
`Ign`'d, the fallback to archive.ubuntu.com stalled mid-fetch, and the job sat
there for six hours before reporting anything. A green run takes ~10 minutes.

Adds a 45m job timeout (generous headroom over a cold-cache run) and a 10m
timeout on the apt step itself, which is the step actually observed to hang.
A stall now surfaces in minutes instead of occupying a runner for six hours.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant