fix(playground): report structured TSL errors #75
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import { createEffect, createMemo, createSignal, on, onCleanup, onMount } from 'solid-js' | ||
| import { buildTslPreviewModule } from '../../../../../packages/schema/src/tsl-preview-module.ts' | ||
| import { collectShaderDiagnostics, diagnosticsToMessages } from '../../lib/webgl-shader-errors' | ||
| import { createPlainErrorReport, createTslErrorReport } from '../../lib/tsl-error-reporting' | ||
| import type { PlaygroundErrorReport } from '../../lib/playground-types' | ||
| import TslPreviewCanvas from '../TslPreviewCanvas' | ||
|
|
||
| type THREE = typeof import('three') | ||
|
|
@@ -11,7 +13,7 @@ type PlaygroundCanvasProps = { | |
| tslSource?: string | ||
| pipeline: string | ||
| language: 'glsl' | 'tsl' | ||
| onError: (report: PlaygroundErrorReport) => void | ||
| onScreenshotReady: (base64: string) => void | ||
| } | ||
|
|
||
|
|
@@ -29,7 +31,7 @@ export default function PlaygroundCanvas(props: PlaygroundCanvasProps) { | |
| try { | ||
| return buildTslPreviewModule(props.tslSource) | ||
| } catch (error) { | ||
| props.onError(createTslErrorReport(error, 'tsl-parse', 'Failed to build TSL preview module.')) | ||
| return '' | ||
| } | ||
| }) | ||
|
|
@@ -172,7 +174,7 @@ export default function PlaygroundCanvas(props: PlaygroundCanvasProps) { | |
| uniforms: shaderUniforms, | ||
| }) | ||
| } catch (e) { | ||
| props.onError(createPlainErrorReport([e instanceof Error ? e.message : 'Shader compilation failed'])) | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -210,21 +212,21 @@ export default function PlaygroundCanvas(props: PlaygroundCanvasProps) { | |
| ? renderError.message | ||
| : 'Shader compilation failed' | ||
|
|
||
| props.onError(createPlainErrorReport( | ||
| shaderDiagnostics.length > 0 ? diagnosticsToMessages(shaderDiagnostics) : [fallbackMessage], | ||
| )) | ||
| return | ||
| } finally { | ||
| renderer.debug.onShaderError = previousShaderError | ||
| } | ||
|
|
||
|
Owner
Author
There was a problem hiding this comment. Worth noting for a future enhancement: GLSL errors go through
Owner
Author
There was a problem hiding this comment. Kept out of scope for this PR, but I filed the follow-up here: #76. That issue tracks mapping ShaderDiagnostic[] through PlaygroundErrorReport so GLSL gets the same structured coverage as TSL. |
||
| if (shaderDiagnostics.length > 0) { | ||
| props.onError(createPlainErrorReport(diagnosticsToMessages(shaderDiagnostics))) | ||
| return | ||
| } | ||
|
|
||
| // No errors — clear any previous errors and capture screenshot | ||
| props.onError(createPlainErrorReport([])) | ||
| captureScreenshot() | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import assert from 'node:assert/strict' | ||
| import { | ||
| createPlainErrorReport, | ||
| createTslErrorReport, | ||
| TslPreviewError, | ||
| } from './tsl-error-reporting.ts' | ||
|
|
||
| function runTest(name: string, callback: () => void) { | ||
| callback() | ||
| console.log(`ok ${name}`) | ||
| } | ||
|
|
||
| runTest('createPlainErrorReport keeps structured errors empty', () => { | ||
| assert.deepEqual(createPlainErrorReport(['plain error']), { | ||
| errors: ['plain error'], | ||
| structuredErrors: [], | ||
| }) | ||
| }) | ||
|
|
||
| runTest('createTslErrorReport preserves explicit preview error kinds', () => { | ||
| const report = createTslErrorReport( | ||
| new TslPreviewError('tsl-material-build', 'createPreview(runtime) must return a material.'), | ||
| 'tsl-runtime', | ||
| 'fallback', | ||
| ) | ||
|
|
||
| assert.deepEqual(report, { | ||
| errors: ['createPreview(runtime) must return a material.'], | ||
| structuredErrors: [{ | ||
| kind: 'tsl-material-build', | ||
| message: 'createPreview(runtime) must return a material.', | ||
| }], | ||
| }) | ||
| }) | ||
|
|
||
| runTest('createTslErrorReport maps SyntaxError to tsl-parse', () => { | ||
| const report = createTslErrorReport( | ||
| new SyntaxError('Unexpected token'), | ||
| 'tsl-runtime', | ||
| 'fallback', | ||
| ) | ||
|
|
||
| assert.deepEqual(report, { | ||
| errors: ['Unexpected token'], | ||
| structuredErrors: [{ | ||
| kind: 'tsl-parse', | ||
| message: 'Unexpected token', | ||
| }], | ||
| }) | ||
| }) | ||
|
Owner
Author
There was a problem hiding this comment. Good coverage of the runTest('createTslErrorReport uses fallbackKind for generic errors', () => {
const report = createTslErrorReport(
new Error('something broke'),
'tsl-runtime',
'fallback',
)
assert.deepEqual(report, {
errors: ['something broke'],
structuredErrors: [{ kind: 'tsl-runtime', message: 'something broke' }],
})
})nit / optional
Owner
Author
There was a problem hiding this comment. Addressed in 19c2091. I added the plain Error coverage so the allbackKind path is now tested directly. |
||
|
|
||
| console.log('tsl-error-reporting tests passed') | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import type { PlaygroundErrorReport, PlaygroundError } from './playground-types' | ||
|
|
||
| type TslErrorKind = Extract<PlaygroundError['kind'], 'tsl-parse' | 'tsl-runtime' | 'tsl-material-build'> | ||
|
|
||
| export class TslPreviewError extends Error { | ||
| kind: TslErrorKind | ||
|
|
||
| constructor(kind: TslErrorKind, message: string) { | ||
| super(message) | ||
| this.kind = kind | ||
| this.name = 'TslPreviewError' | ||
| } | ||
| } | ||
|
|
||
| function getMessage(error: unknown, fallbackMessage: string): string { | ||
| if (error instanceof Error && error.message.trim()) { | ||
| return error.message.trim() | ||
| } | ||
|
|
||
| if (typeof error === 'string' && error.trim()) { | ||
| return error.trim() | ||
| } | ||
|
|
||
| return fallbackMessage | ||
| } | ||
|
|
||
| export function createPlainErrorReport(errors: string[]): PlaygroundErrorReport { | ||
| return { | ||
| errors, | ||
| structuredErrors: [], | ||
| } | ||
| } | ||
|
|
||
| export function createTslErrorReport( | ||
| error: unknown, | ||
| fallbackKind: TslErrorKind, | ||
| fallbackMessage: string, | ||
| ): PlaygroundErrorReport { | ||
| const message = getMessage(error, fallbackMessage) | ||
| const kind = error instanceof TslPreviewError | ||
| ? error.kind | ||
| : error instanceof SyntaxError | ||
| ? 'tsl-parse' | ||
| : fallbackKind | ||
|
|
||
| return { | ||
| errors: [message], | ||
| structuredErrors: [{ kind, message }], | ||
| } | ||
|
Comment on lines
+34
to
+49
Owner
Author
There was a problem hiding this comment. Clean classification chain. The One thing to be aware of:
Owner
Author
There was a problem hiding this comment. Addressed in 19c2091. playground-db.ts now uses the shared PlaygroundErrorReport type directly instead of keeping a separate ErrorReportPayload alias. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit roundabout — you construct a
TslPreviewErrorjust socreateTslErrorReportcan unwrap it back into the same kind and message. The message string appears three times.Since the kind and message are fully known here, you could skip the round-trip:
Or if you prefer keeping the helper for consistency, at least the
TslPreviewErrorwrapper carries the info without needing the redundant string args:nit — works correctly as-is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 19c2091. I added a direct createKnownTslErrorReport() helper and switched the WebGPU gate to use it, so there is no redundant TslPreviewError round-trip and the message only lives in one place.