From c75ee145d3986208105eb42b7da547b3a52160e8 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Thu, 4 Jun 2026 12:46:23 -0400 Subject: [PATCH] fix(text): pass error (not texture) to SDF font 'failed' handler EventEmitter invokes listeners as (target, data), so the error payload from a Texture's 'failed' event is the second argument. The SDF font loader's handler read it as a single parameter, so it rejected the font-load promise and logged with the Texture instance instead of the actual TextureError. The font still failed correctly, but the rejection value and console output were the wrong object, hurting diagnostics for SDF font load failures. The bogus `(error: Error)` annotation also masked it, since `on()`'s listener type is `(target: any, data: any)`. Fix the handler to `(_target, error: TextureError)` and add a regression test that drives loadFont to the failed branch and asserts it rejects with the emitted error rather than the emitting texture. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/text-rendering/SdfFontHandler.ts | 7 +- .../tests/SdfFontHandler.test.ts | 112 ++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/core/text-rendering/tests/SdfFontHandler.test.ts diff --git a/src/core/text-rendering/SdfFontHandler.ts b/src/core/text-rendering/SdfFontHandler.ts index e665234..0c7b555 100644 --- a/src/core/text-rendering/SdfFontHandler.ts +++ b/src/core/text-rendering/SdfFontHandler.ts @@ -12,6 +12,7 @@ import { UpdateType } from '../CoreNode.js'; import { hasZeroWidthSpace } from './Utils.js'; import { normalizeFontMetrics } from './TextLayoutEngine.js'; import { isProductionEnvironment } from '../../utils.js'; +import type { TextureError } from '../TextureError.js'; /** * SDF Font Data structure matching msdf-bmfont-xml output @@ -397,7 +398,11 @@ export const loadFont = ( resolve(); }); - atlasTexture.on('failed', (error: Error) => { + // EventEmitter invokes listeners as (target, data), so the error payload + // is the SECOND argument. The first arg is the Texture that emitted the + // event. Reading it as the only param (the previous behavior) rejected + // and logged the Texture instead of the actual TextureError. + atlasTexture.on('failed', (_target, error: TextureError) => { // Cleanup on error fontLoadPromises.delete(fontFamily); if (fontCache[fontFamily]) { diff --git a/src/core/text-rendering/tests/SdfFontHandler.test.ts b/src/core/text-rendering/tests/SdfFontHandler.test.ts new file mode 100644 index 0000000..d0e8726 --- /dev/null +++ b/src/core/text-rendering/tests/SdfFontHandler.test.ts @@ -0,0 +1,112 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { loadFont } from '../SdfFontHandler.js'; +import { EventEmitter } from '../../../common/EventEmitter.js'; +import { TextureError, TextureErrorCode } from '../../TextureError.js'; +import type { Stage } from '../../Stage.js'; + +// Minimal XHR stand-in: synchronously delivers valid SDF font JSON so loadFont +// proceeds to create the atlas texture and attach its event listeners. +class FakeXHR { + status = 200; + response: unknown = null; + responseType = ''; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + open(): void {} + send(): void { + this.response = { chars: [{}] }; + if (this.onload !== null) { + this.onload(); + } + } +} + +// A texture is just an EventEmitter to loadFont; stub the few props it reads. +function makeFakeTexture() { + const tex = new EventEmitter() as unknown as EventEmitter & { + state: string; + preventCleanup: boolean; + setRenderableOwner: (owner: string, val: boolean) => void; + }; + tex.state = 'loading'; // not 'loaded' -> goes through the listener path + tex.preventCleanup = false; + tex.setRenderableOwner = () => {}; + return tex; +} + +// Drain microtasks + one macrotask so the async loader reaches listener setup. +const flush = (): Promise => new Promise((r) => setTimeout(r, 0)); + +describe('SdfFontHandler loadFont — failed event argument', () => { + let errSpy: ReturnType; + let originalXHR: unknown; + + beforeEach(() => { + errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + originalXHR = (globalThis as unknown as { XMLHttpRequest: unknown }) + .XMLHttpRequest; + (globalThis as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest = + FakeXHR; + }); + + afterEach(() => { + errSpy.mockRestore(); + (globalThis as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest = + originalXHR; + }); + + it('rejects with the TextureError (second emit arg), not the emitting texture', async () => { + const tex = makeFakeTexture(); + const stage = { + txManager: { createTexture: () => tex }, + } as unknown as Stage; + + const promise = loadFont(stage, { + fontFamily: 'TestSdfFailFont', + atlasUrl: 'atlas.png', + atlasDataUrl: 'atlas.json', + } as Parameters[1]); + + await flush(); + + const error = new TextureError( + TextureErrorCode.TEXTURE_UPLOAD_FAILED, + 'boom', + ); + + // Attach the rejection assertion before emitting so the handler is ready. + const assertion = expect(promise).rejects.toBe(error); + + // EventEmitter calls listeners as (target, data) -> (tex, error). + tex.emit('failed', error); + + await assertion; + }); + + it('logs the error, not the texture, on failure', async () => { + const tex = makeFakeTexture(); + const stage = { + txManager: { createTexture: () => tex }, + } as unknown as Stage; + + const promise = loadFont(stage, { + fontFamily: 'TestSdfFailFontLog', + atlasUrl: 'atlas.png', + atlasDataUrl: 'atlas.json', + } as Parameters[1]); + + await flush(); + + const error = new TextureError( + TextureErrorCode.TEXTURE_UPLOAD_FAILED, + 'boom', + ); + const rejected = promise.catch(() => {}); + tex.emit('failed', error); + await rejected; + + const lastCall = errSpy.mock.calls[errSpy.mock.calls.length - 1]!; + expect(lastCall[1]).toBe(error); + expect(lastCall[1]).not.toBe(tex); + }); +});