-
-
Notifications
You must be signed in to change notification settings - Fork 10
fix: close factory, config, and playground liveness gaps #1024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2302ee6
7fe88d3
4a84a96
e8e8da5
1efc874
d944e35
ce74753
bb2690d
6b0a2cf
a8e7bf6
764b5a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| export const CONFIG_EVALUATOR_MAX_BUFFER = 16 * 1024 * 1024; | ||
| export const CONFIG_EVALUATOR_TIMEOUT_MS = 60_000; | ||
| export const CONFIG_EVALUATOR_TEARDOWN_GRACE_MS = 5_000; | ||
| export const CONFIG_EVALUATOR_STATUS_FD = 3; | ||
| export const CONFIG_EVALUATOR_PROCESS_OPTIONS = Object.freeze({ | ||
| killSignal: "SIGKILL" as const, | ||
| maxBuffer: CONFIG_EVALUATOR_MAX_BUFFER, | ||
| timeout: CONFIG_EVALUATOR_TIMEOUT_MS + CONFIG_EVALUATOR_TEARDOWN_GRACE_MS, | ||
| }); | ||
|
|
||
| interface ConfigEvaluatorProcessResult { | ||
| error?: Error; | ||
| output?: readonly (string | null)[] | null; | ||
| signal: NodeJS.Signals | null; | ||
| status: number | null; | ||
| stderr: string | null | undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Classify the ways the isolated lint-config evaluator can stop. | ||
| * | ||
| * Node reports both timeout and max-buffer termination with the configured | ||
| * signal, so the process error code must take precedence over the signal. The | ||
| * evaluator uses `SIGKILL`: Node's synchronous process API otherwise keeps | ||
| * waiting when a POSIX child handles the default `SIGTERM` without exiting. A | ||
| * bare signal is an external termination and a non-zero status is an evaluator | ||
| * failure whose stderr tail contains the useful user-config diagnostic. | ||
| */ | ||
| export function configEvaluatorProcessFailure( | ||
| result: ConfigEvaluatorProcessResult, | ||
| configPath: string, | ||
| ): Error | undefined { | ||
| const code = (result.error as NodeJS.ErrnoException | undefined)?.code; | ||
| const nestedCode = result.output?.[CONFIG_EVALUATOR_STATUS_FD]?.trim() ?? ""; | ||
| if (code === "ETIMEDOUT" || nestedCode === "ETIMEDOUT") { | ||
| return new Error( | ||
| `@ttsc/lint: ttsx evaluation of ${configPath} timed out after ` + | ||
| `${CONFIG_EVALUATOR_TIMEOUT_MS / 1_000} seconds. ` + | ||
| "Simplify the config or move heavy work out of top-level.", | ||
| ); | ||
| } | ||
| if (code === "ENOBUFS" || nestedCode === "ENOBUFS") { | ||
| return new Error( | ||
| `@ttsc/lint: ttsx evaluation of ${configPath} exceeded the ` + | ||
| `${CONFIG_EVALUATOR_MAX_BUFFER / (1024 * 1024)} MiB output limit. ` + | ||
| "Reduce console output from the config and its dependencies.", | ||
| ); | ||
| } | ||
| if (result.error) { | ||
| return new Error( | ||
| `@ttsc/lint: failed to spawn ttsx for ${configPath}: ${result.error.message}`, | ||
| ); | ||
| } | ||
| if (result.signal) { | ||
| return new Error( | ||
| `@ttsc/lint: ttsx evaluation of ${configPath} was killed by signal ${result.signal}.`, | ||
| ); | ||
| } | ||
| if (result.status !== 0) { | ||
| const reason = configEvaluatorFailureReason(result.stderr); | ||
| return new Error( | ||
| `@ttsc/lint: lint config ${configPath} evaluation failed with exit code ${String(result.status)}` + | ||
| (reason === "" ? "" : "\n" + reason), | ||
| ); | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Pass the semantic deadline and private status pipe through the `ttsx` wrapper | ||
| * to the runtime child that actually executes the config. | ||
| */ | ||
| export function configEvaluatorBoundaryEnvironment( | ||
| now: number = Date.now(), | ||
| ): NodeJS.ProcessEnv { | ||
| return { | ||
| TTSC_TTSX_EVALUATOR_DEADLINE_MS: String(now + CONFIG_EVALUATOR_TIMEOUT_MS), | ||
| TTSC_TTSX_EVALUATOR_MAX_BUFFER_BYTES: String(CONFIG_EVALUATOR_MAX_BUFFER), | ||
| TTSC_TTSX_EVALUATOR_STATUS_FD: String(CONFIG_EVALUATOR_STATUS_FD), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Return the useful tail of evaluator stderr without turning an exception into | ||
| * an unbounded duplicate of the already-forwarded child stream. | ||
| */ | ||
| function configEvaluatorFailureReason( | ||
| stderr: string | null | undefined, | ||
| ): string { | ||
| const bounded = (stderr ?? "").slice(-CONFIG_EVALUATOR_REASON_MAX_CHARS); | ||
| const lines = bounded | ||
| .split(/\r?\n/) | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: limiting the number of lines does not limit one giant line. |
||
| .map((line) => line.trimEnd()) | ||
| .filter((line) => line.trim() !== ""); | ||
| return lines.slice(-CONFIG_EVALUATOR_REASON_LINES).join("\n"); | ||
| } | ||
|
|
||
| const CONFIG_EVALUATOR_REASON_LINES = 5; | ||
| const CONFIG_EVALUATOR_REASON_MAX_CHARS = 8 * 1024; | ||
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.
P1 (merge-blocking at this commit):
SIGKILLmakes this particularspawnSyncreturn, but this process is only the ttsx wrapper.runPreparedEntrycreates the Node process that actually executes the config/descriptor, and killing the wrapper does not kill that nested runtime on POSIX. The semantic deadline and output cap must be applied to the runtime child itself, with its cause propagated back to this parent.