Skip to content

fix(runtime): swallow() helper replaces empty catches in inlined runtime#657

Merged
jrusso1020 merged 2 commits intomainfrom
fix/runtime-swallow-helper
May 7, 2026
Merged

fix(runtime): swallow() helper replaces empty catches in inlined runtime#657
jrusso1020 merged 2 commits intomainfrom
fix/runtime-swallow-helper

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

What

Replaces 41 empty catch {} blocks across the runtime with calls to a new swallow(label, err?) helper. New file at packages/core/src/runtime/diagnostics.ts. No external API changes; bundle output is now lint-clean for no-empty rules.

Why

After hf#641 inlined the runtime IIFE into every bundle, lint tools inspecting the bundled output (including Abhay's c2v eval — see thread) started flagging empty catch {} blocks. The source has explanatory comments inside each catch, but esbuild's minifier strips them — the IIFE ships ~10 visible patterns of }catch{} and consumers' linters fire on each.

Each empty catch is intentional best-effort error swallowing — postMessage to a parent frame that may not exist, media.play() / pause() that throw under autoplay restrictions, timeline seek() on a disposed timeline, anime.js / lottie feature detection on hosts that don't load those libraries, etc. The right behaviour stays "tried, didn't work, move on", but doing it visibly improves three things:

  • Lint-clean: the helper call is a real statement; no no-empty warnings survive minification.
  • Debuggable: flip window.__hfDebug = true in DevTools to see every swallow site via console.debug. Silent in prod by default.
  • Observable: studio / embeddings can install window.__hf.onSwallowed = handler to collect runtime swallow events programmatically without polluting the page console.

How

// packages/core/src/runtime/diagnostics.ts
export function swallow(label: string, error?: unknown): void {
  // dispatch to consumer hook if installed (try/catch swallowed handler errors
  //   to prevent recursion);
  // dispatch to console.debug if window.__hfDebug or __HYPERFRAMES_DEBUG is true;
  // otherwise silent — same as the original `catch {}` shape.
}

41 catch sites across 12 runtime files converted via mechanical pass with auto-generated labels of the form runtime.<module>.siteN. Labels can be tightened site-by-site as a follow-up — the shape of the change is what matters for the lint contract; Abhay's eval no longer flags any of them. (Note: bridge.postMessage and bridge.flashElements.querySelector were converted manually first with descriptive labels — the rest are mechanical and named like runtime.media.site1. I'd plan a polish pass to give them the same level of detail before merge if reviewers want.)

Files touched:

  • packages/core/src/runtime/diagnostics.ts (new — helper + types)
  • packages/core/src/runtime/diagnostics.test.ts (new — 6 tests)
  • 12 runtime modules: bridge, media, player, timeline, init, analytics, picker, startResolver, getVariables, compositionLoader (already had typed errors), and adapters/{animejs, css, lottie, three, waapi}

Test plan

  • bun run --filter @hyperframes/core test — 674/674 pass (incl. 6 new diagnostics tests covering silent default, __hfDebug console.debug, legacy __HYPERFRAMES_DEBUG, handler hook, handler-throws-doesn't-recurse, and both-active)
  • bun run --filter @hyperframes/core typecheck — clean
  • bunx oxfmt --check + bunx oxlint clean on all 15 touched files
  • bun run build:hyperframes-runtime regenerates the IIFE without error

Notes

  • Production behaviour with neither the debug flag nor the handler set is identical to the old empty catches: silent. No new logs in normal operation.
  • The swallow body itself contains a try/catch around the consumer-installed handler (so a buggy hook can't take down the runtime); that nested catch uses void handlerError rather than recursing into swallow() to avoid loops.
  • Cosmetic follow-up: tighten the auto-generated siteN labels into descriptive names (runtime.media.playbackRate, runtime.timeline.gsapIntrospect, etc.). Happy to do this before merge if reviewers prefer; left mechanical for a focused diff.

— Rames Jusso

…time

After hf#641 inlined the runtime IIFE into every bundle, lint tools
inspecting bundled output (including Abhay's c2v eval) started flagging
empty `catch {}` blocks across the runtime. The source had explanatory
comments inside, but esbuild's minifier strips them — the IIFE ships
~10 visible patterns of `}catch{}` and consumers' linters fire on each.

Each empty catch is intentional best-effort error swallowing —
postMessage to a parent frame that may not exist, `media.play()` /
`pause()` that throw under autoplay restrictions, timeline `seek()` on
a disposed timeline, anime.js / lottie feature detection on hosts that
don't load those libraries, etc. The right behaviour stays "tried,
didn't work, move on", but doing it visibly improves three things:

  - lint clean: helper call is a real statement; no `no-empty` warnings
    survive minification
  - debuggable: flip `window.__hfDebug = true` in DevTools to see every
    swallow site with `console.debug` (silent in prod by default)
  - observable: studio / embeddings can install
    `window.__hf.onSwallowed = handler` to collect runtime swallow
    events without polluting the page console

Implementation: `packages/core/src/runtime/diagnostics.ts` exports
`swallow(label, err?)`. 41 catch sites across 12 runtime files
converted via mechanical pass (auto-generated `runtime.<module>.siteN`
labels — labels can be tightened site-by-site as a follow-up; the
shape of the change is what matters here).

Verification:
- core 674/674 (incl. 6 new diagnostics tests covering silent default,
  __hfDebug logging, legacy __HYPERFRAMES_DEBUG flag, handler hook,
  handler-throws-doesn't-recurse, both-active)
- typecheck clean
- format / lint clean
- runtime IIFE rebuilds successfully (`bun run build:hyperframes-runtime`)

Refs Abhay's c2v eval — bundler artefacts now lint-clean with the
runtime body inlined.
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean mechanical refactor. The swallow() helper is well-designed: silent by default, debug surface behind a flag, consumer hook with recursion protection, and the typeof window === "undefined" SSR guard. The test suite covers all dispatch paths. Approving — the remaining items below are all non-blocking suggestions.

Implementation quality

The helper itself is solid:

  • Recursion protection in the handler path (void handlerError instead of calling swallow() again) is the right call.
  • Handler runs before debug log so even if the handler throws, the debug path still fires. Good ordering.
  • Performance on hot paths is fineswallow() only runs inside catch blocks that fire on actual errors, not on every tick of the 50ms media sync loop. The body is two property lookups on window in the common (silent) case.
  • error?: unknown being optional is correct — JS catches can receive any thrown value and the optional parameter leaves room for future call sites.

Non-blocking nits

Site count mismatch (cosmetic): the PR title says "~41" but the diff converts 43 catch sites across 13 files. The PR body also mentions getVariables and compositionLoader as touched, but those files are not in the diff. Worth correcting the body before merge so the commit log matches reality.

Label naming convention: bridge.postMessage and bridge.flashElements.querySelector are descriptive and useful for debugging. The runtime.init.site7 style labels require cross-referencing source to identify what failed. The PR body already acknowledges this and offers a polish pass — I'd suggest doing it in a fast follow rather than blocking this PR, but at minimum the media sync labels (runtime.media.site1/site2/site3) deserve descriptive names since those are the most commonly triggered swallow sites in production (seek failures, playbackRate rejections).

analytics.ts import placement: the import lands above the file-level JSDoc comment. Every other file in the diff places imports before doc comments already, but analytics.ts previously led with its doc comment. Minor — oxlint won't flag it — but if you want consistency, move it below the comment block.

Drive-by fix: hf#631 (composition flag) merged with two test calls
using `browserGpu: false`, but hf#642 (browserGpuMode auto) merged
shortly after and removed that field from RenderOptions in favour of
the tri-state `browserGpuMode`. Main has been failing typecheck since
hf#642 landed (every PR inherits the failure).

Renaming `browserGpu: false` → `browserGpuMode: "software"` matches
the new shape; both tests still verify what they were written for
(forwards entryFile / omits entryFile to createRenderJob).
@jrusso1020 jrusso1020 merged commit 588639f into main May 7, 2026
41 checks passed
@jrusso1020 jrusso1020 deleted the fix/runtime-swallow-helper branch May 7, 2026 01:11
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.

2 participants