coder / coder/ghostty-web

A single throw inside render() permanently stops the render loop, and a consumer cannot restart it

Open
#189 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
2.8k
Forks
172
PR merge metrics
No merged PRs in 30d

Description

Hi! First off, thanks for ghostty-web — we run it as the terminal in [dev-3.0](https://github.com/h0x91b/dev-3.0) and it's been a pleasure to work with.

We hit a failure mode in production that I think is worth reporting separately from the individual crashes, because it turns *any* exception in the render path into a permanently dead terminal that the embedding app cannot recover from.

## What happens

The pane goes blank and stays blank. The PTY, the shell and the process keep running, the DOM is fine, keyboard input still reaches the shell — nothing repaints, ever. Neither resizing the window nor toggling fullscreen brings it back. Only recreating the `Terminal` (or reloading the page) helps.

## Why it is permanent

`lib/terminal.ts` → `startRenderLoop()` (current `main`):

```ts
private startRenderLoop(): void {
if (this.animationFrameId) return;
const loop = () => {
if (!this.isDisposed && this.isOpen) {
this.renderer!.render(this.wasmTerm!, false, this.viewportY, this, this.scrollbarOpacity);
// ...
this.animationFrameId = requestAnimationFrame(loop);
}
};
loop();
}
```

The next frame is scheduled only *after* `render()` returns, and the loop body has no `try`/`catch`. So one throw out of `render()` means no further frames — for the lifetime of that `Terminal`. And since `startRenderLoop` is `private`, an embedder has no way to restart it: there is no public "resume rendering" entry point and no error event to hook.

## What we actually saw

Two different exceptions, both reaching us through `term.resize(cols, rows)`, both leaving a permanently blank pane. From our logs (ghostty-web 0.4.0, macOS, Bun-based Electrobun app, WKWebView):

```
ERROR [refit] term.resize threw {"error":"RangeError: Arguments contain a value that is out of range of code points","cols":257,"rows":82}
ERROR [refit] term.resize threw {"error":"RuntimeError: Out of bounds memory access (evaluating 'this.exports.ghostty_terminal_resize(this.handle,e,t)')","cols":158,"rows":82}
```

In the second case the trap repeated ~116 times at frame cadence (a scrollbar fade animation kept calling `render()`), and eventually `ghostty_terminal_resize` itself trapped — so that WASM terminal was gone, not just one frame. Both were triggered by an ordinary resize: one right after we reattached to an existing PTY, one from a user dragging a side panel.

The first one has a plausible source that is still present on `main` — `lib/renderer.ts:648`:

```ts
char = String.fromCodePoint(cell.codepoint || 32); // Default to space if null
```

no range check, while the xterm-compat `getChars()` in the same codebase does guard the same value:

```ts
codepoint < 0 || codepoint > 0x10ffff || (codepoint >= 0xd800 && codepoint <= 0xdfff) ? "�" : String.fromCodePoint(codepoint)
```

So a garbage cell (for us most likely read around a resize realloc) is a replacement character in one path and a fatal, terminal-killing throw in the other.

## Related, but not the same thing

- #132 fixes the resize/realloc race itself (pausing the loop around the WASM resize) — that removes one source of throws, and it is exactly what we are missing. We are on the latest npm release, 0.4.0, so we don't have it; I see #137 and #182 already cover getting a release out, so I'm not asking again here.
- #141 is another route to the same `Out of bounds memory access`, from `free()` after multi-codepoint graphemes.

This report is about the layer above both: whatever the cause, one bad frame should not be able to end rendering forever with no way back.

## What would help

1. **Survive a bad frame.** Reschedule the next frame in a `finally` (or wrap the body in `try`/`catch`), so an exception costs one frame instead of the terminal. Failing loudly is fine and welcome — logging or an `onRenderError` event would be better than silence — as long as the loop stays alive.
2. **Give embedders a way back.** A public way to restart rendering (or a documented "the renderer is dead" signal) would let an app recover without throwing away the `Terminal`. Today our only option is to dispose the terminal, build a new one and repaint from scratch, which is what we ended up shipping.
3. **Range-guard the draw path** in `renderer.ts` the way `getChars()` already does, so a bad codepoint degrades to `�` instead of killing the terminal.

Happy to test a patch against our app — we see this in the wild often enough to tell quickly whether it's gone. Thanks again!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in lib/terminal.ts at startRenderLoop() and inspect lib/renderer.ts:648 alongside the existing xterm-compat getChars() range guard. Reproduce the failure around term.resize(), then determine how the loop should behave after a render exception and what public recovery or error signal is appropriate. Done means a bad frame no longer permanently blanks the terminal and invalid codepoints do not reach String.fromCodePoint unguarded.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript, wasm
Domain
frontend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.