1
0
mirror of https://github.com/alexandrev/xslt-lab.git synced 2026-09-13 08:43:16 +00:00

Fix: give the backend room to run, and stop transforming half-typed expressions

The backend container runs three JVMs — Saxon 12, Saxon 9.6 and Xalan — whose
-Xmx values add up to 512 MB, inside a container limited to 256Mi. The limit was
half the heap the JVMs believed they had, so the kernel was killing the
container: OOMKilled twice a day on the 16th and 17th of August, five times a
day on the 18th and 19th, and 81 users in 24 hours got "transform service
unavailable" instead of a result. Before each kill the serial collector spends
hours in continuous full GC, which is why three of the five pods sat pinned at
exactly 0.5 CPU — their limit — around the clock, and why XSLT 2.0's p95 was
5.5s against 0.7s for the other two engines.

Those limits were never in the chart to begin with: the backend deployment has
never rendered a resources block, values.resources.backend was declared and
unused, and the live numbers came from a manual kubectl patch that an ArgoCD
sync would have silently dropped. So the chart now owns them, with room for
what actually runs in there. The CPU request moves 10m -> 250m as well: against
real usage of 500m the HPA read utilisation in the thousands of percent and the
deployment sat permanently at maxReplicas, unable to signal anything. And the
backend gets minReplicas 2, for the reason the frontend already learnt.

The other half is the mid-keystroke problem again, one level down. Gating the
automatic run on well-formedness took the error rate from 54% to 24%, but a
stylesheet whose XPath is half-typed is perfectly good XML, so it still went to
the backend and still came back as a compile error. A day of logs is mostly
that, keystroke by keystroke: 'current()/..[@N' then '[@Na' then '[@Name', and
"Required attribute 'select' is missing" 65 times on a single line. So the gate
now also declines to send an expression that is provably unfinished — a
required attribute that is absent, an empty value, a quote or bracket still
open, a trailing token that cannot end an expression. Facts about the text, not
guesses at intent: there are twice as many tests for what it must leave alone
as for what it catches, including every template the app itself ships. It is
skipped above 64KB, where a document was pasted rather than typed, and as
before it only holds back the automatic run — "Run it anyway" is always there.

Finally, the error classifier learns the rest of what was sitting in "other":
Xalan's "Syntax error in '<expr>'" is the stylesheet's XPath, not the input.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HnUwBeXv6PCTEzRpFoMSU
This commit is contained in:
2026-08-19 19:30:02 +00:00
parent 9e81475957
commit 4e1f3ffdac
9 changed files with 428 additions and 3 deletions
+8
View File
@@ -173,6 +173,14 @@ func classifyTransformError(msg string) (code, class string) {
"is undefined", "is undefined",
"format-number picture", "format-number picture",
"cannot find external method", "cannot find external method",
// Second pass over what was still sitting in "other" (2026-08-19). The
// XPath ones dominate: Xalan reports a bad path expression as "Syntax
// error in '<expr>'", which is the stylesheet's XPath, not the input.
"syntax error in '",
"does not appear to be a stylesheet",
"must precede all other element children",
"has not been defined",
"a sequence of more than one item is not allowed",
} }
switch { switch {
+6
View File
@@ -235,6 +235,12 @@ func TestClassifyTransformError(t *testing.T) {
{"format-number picture", "format-number picture: Passive character must not appear between active characters in a sub-picture", "stylesheet", "COMPILE"}, {"format-number picture", "format-number picture: Passive character must not appear between active characters in a sub-picture", "stylesheet", "COMPILE"},
{"missing java extension", "Cannot find external method 'com.example.util.DateUtil.now' (must be public).", "stylesheet", "COMPILE"}, {"missing java extension", "Cannot find external method 'com.example.util.DateUtil.now' (must be public).", "stylesheet", "COMPILE"},
{"stray xml declaration", `The processing instruction target matching "[xX][mM][lL]" is not allowed.`, "input_xml", "PARSE"}, {"stray xml declaration", `The processing instruction target matching "[xX][mM][lL]" is not allowed.`, "input_xml", "PARSE"},
// Second pass over "other", from the 2026-08-19 log review.
{"xalan xpath syntax", `Syntax error in 'current()/..[@Name = 'Programming''.`, "stylesheet", "COMPILE"},
{"not a stylesheet", "The supplied file does not appear to be a stylesheet", "stylesheet", "COMPILE"},
{"misplaced xsl:import", "The xsl:import element children must precede all other element children of an xsl:stylesheet element, including any xsl:include element children.", "stylesheet", "COMPILE"},
{"undeclared key", "Key 'person-key' has not been defined", "stylesheet", "COMPILE"},
{"sequence where one item expected", `A sequence of more than one item is not allowed as the first argument of substring() ("a", "b", ...)`, "stylesheet", "COMPILE"},
// A limit the service imposes — a bug candidate, not the user's mistake. // A limit the service imposes — a bug candidate, not the user's mistake.
{"xpath operator limit", "JAXP0801002: the compiler encountered an XPath expression containing '101' operators that exceeds the '100' limit set by 'FEATURE_SECURE_PROCESSING'.", "backend", "XPATH_OP_LIMIT"}, {"xpath operator limit", "JAXP0801002: the compiler encountered an XPath expression containing '101' operators that exceeds the '100' limit set by 'FEATURE_SECURE_PROCESSING'.", "backend", "XPATH_OP_LIMIT"},
{"truly unknown", "some unexpected failure", "other", "OTHER"}, {"truly unknown", "some unexpected failure", "other", "OTHER"},
+1 -1
View File
@@ -1,6 +1,6 @@
apiVersion: v2 apiVersion: v2
name: xslt-playground name: xslt-playground
description: Helm chart for xslt-playground frontend and backend description: Helm chart for xslt-playground frontend and backend
version: 0.4.1 version: 0.4.2
appVersion: "0.1.0" appVersion: "0.1.0"
@@ -42,6 +42,7 @@ spec:
mountPath: {{ .Values.firebase.credentialsMountPath }} mountPath: {{ .Values.firebase.credentialsMountPath }}
readOnly: true readOnly: true
{{- end }} {{- end }}
resources: {{- toYaml .Values.resources.backend | nindent 12 }}
ports: ports:
- containerPort: {{ .Values.service.backend.port }} - containerPort: {{ .Values.service.backend.port }}
name: http name: http
+17 -2
View File
@@ -33,7 +33,20 @@ ingress:
resources: resources:
frontend: {} frontend: {}
backend: {} # The backend container runs three JVMs side by side (Saxon 12, Saxon 9.6 and
# Xalan — see backend/start.sh), whose -Xmx values alone add up to 512 MB,
# before metaspace, thread stacks and the Go server. The limit has to leave
# room for all of it or the kernel kills the container mid-transform.
backend:
requests:
# Sized so the HPA reads something meaningful: with a 10m request the
# utilisation ratio is permanently in the thousands of percent and the
# deployment sits pinned at maxReplicas.
cpu: 250m
memory: 384Mi
limits:
cpu: "1"
memory: 1Gi
hpa: hpa:
frontend: frontend:
@@ -43,7 +56,9 @@ hpa:
targetCPUUtilizationPercentage: 80 targetCPUUtilizationPercentage: 80
backend: backend:
enabled: true enabled: true
minReplicas: 1 # Two, so a single evicted or restarting pod never leaves the app with no
# backend at all — the frontend learnt this the hard way.
minReplicas: 2
maxReplicas: 5 maxReplicas: 5
targetCPUUtilizationPercentage: 80 targetCPUUtilizationPercentage: 80
+20
View File
@@ -16,6 +16,7 @@ import {
checkWellFormed, checkWellFormed,
} from "./lib/workspaceUtils"; } from "./lib/workspaceUtils";
import { templateToWorkspace, findTemplate, STARTER_STYLESHEET } from "./lib/templates"; import { templateToWorkspace, findTemplate, STARTER_STYLESHEET } from "./lib/templates";
import { findUnfinishedExpression } from "./lib/unfinishedExpression";
import { reviewWorkspace } from "./lib/reviewRules"; import { reviewWorkspace } from "./lib/reviewRules";
import { diffLines } from "./lib/diffUtils"; import { diffLines } from "./lib/diffUtils";
import { encodeCompact, decodeCompact, toSharePayload, fromSharePayload, saveFiddle, loadFiddle } from "./lib/shareLink"; import { encodeCompact, decodeCompact, toSharePayload, fromSharePayload, saveFiddle, loadFiddle } from "./lib/shareLink";
@@ -1776,6 +1777,25 @@ export default function App() {
})); }));
return undefined; return undefined;
} }
// Well-formed XML is not the same as a finished expression. A stylesheet
// whose select= is still being typed parses fine and fails to compile, so
// it used to go to the backend anyway — most of what survived the check
// above is exactly that. Same deal: reported here, nothing sent, and the
// explicit run always wins.
const unfinished = findUnfinishedExpression(xsltText);
if (unfinished) {
const text = `Still being typed? ${unfinished.message}.`;
updateWorkspaceStatus(activeTab.id, (prev) => ({
...prev,
error: text,
errorLines: parseErrorLines(text),
isServerError: false,
notWellFormed: true,
isRunning: false,
}));
return undefined;
}
} }
runTransform(xsltText, activeTab.version, activeTab.params, activeTab.id); runTransform(xsltText, activeTab.version, activeTab.params, activeTab.id);
+14
View File
@@ -208,6 +208,20 @@ describe("well-formedness gate", () => {
await waitFor(() => expect(fetch).toHaveBeenCalled(), { timeout: 3000 }); await waitFor(() => expect(fetch).toHaveBeenCalled(), { timeout: 3000 });
}, 10000); }, 10000);
it("does not call the backend while an expression is still being typed", async () => {
// Well-formed XML, unfinished XPath: this is what used to get through.
seedWorkspace(
'<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"><xsl:template match="/"><xsl:value-of select="/Shop/Category/"/></xsl:template></xsl:stylesheet>',
);
render(<App />);
fireEvent.pointerDown(window);
await waitFor(
() => expect(screen.getByText(/still being typed/i)).toBeInTheDocument(),
{ timeout: 3000 },
);
expect(fetch).not.toHaveBeenCalled();
}, 10000);
it("offers a way to run it anyway", async () => { it("offers a way to run it anyway", async () => {
seedWorkspace('<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"><xsl:template match="/'); seedWorkspace('<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"><xsl:template match="/');
render(<App />); render(<App />);
+201
View File
@@ -0,0 +1,201 @@
// The other half of the mid-keystroke problem.
//
// checkWellFormed() stops a document that is not yet valid XML, which removed
// about half the wasted round-trips. What it cannot see is a stylesheet that
// parses perfectly well and carries an XPath expression the user is still in
// the middle of typing: `<xsl:value-of select="/Shop/Category/">` is
// well-formed XML and a compile error, so it went to the backend anyway. A day
// of production logs (2026-08-19) is mostly this, keystroke by keystroke:
//
// Syntax error in 'current()/..[@N' → '[@Na' → '[@Name' → '[@Name = '
// line 15: Required attribute 'select' is missing. (65 times, one line)
// XPST0003: The expression is empty
//
// Everything below is a fact about the expression, never a guess about intent:
// an attribute that is required and absent, a value that is empty, a quote or
// bracket that is still open, or a trailing token that cannot legally end an
// expression. Anything we are not certain about runs, exactly as before — and
// like the well-formedness gate, this only holds back the automatic run, never
// an explicit one.
const XSLT_NS = "http://www.w3.org/1999/XSL/Transform";
// Elements that cannot compile without the listed attribute. xsl:value-of is
// the exception: XSLT 2.0 lets a sequence constructor stand in for select, so
// it only counts as missing when the element is also empty.
const REQUIRED = {
"value-of": "select",
"for-each": "select",
"for-each-group": "select",
"copy-of": "select",
if: "test",
when: "test",
};
// Attributes holding an XPath expression or a match pattern.
const XPATH_ATTRS = [
"select",
"test",
"match",
"use",
"group-by",
"group-adjacent",
];
// Tokens that cannot be the last thing in a complete expression. Word
// operators are only counted when something precedes them, so `select="and"`
// — a perfectly good path selecting <and> children — is left alone.
const TRAILING_SYMBOLS = [
"//",
"/",
"[",
"(",
",",
"@",
"$",
"::",
":",
"=",
"!=",
"<=",
">=",
"<",
">",
"+",
"-",
"|",
"!",
];
const TRAILING_WORDS = [
"and",
"or",
"div",
"idiv",
"mod",
"to",
"eq",
"ne",
"lt",
"gt",
"le",
"ge",
"is",
"instance",
"of",
"as",
"castable",
"cast",
"treat",
"return",
"in",
"satisfies",
"then",
"else",
];
// Walks the value once, tracking string literals so brackets inside a quoted
// string are not mistaken for structure.
function scan(value) {
let quote = null;
let round = 0;
let square = 0;
for (const ch of value) {
if (quote) {
if (ch === quote) quote = null;
continue;
}
if (ch === "'" || ch === '"') {
quote = ch;
} else if (ch === "(") {
round += 1;
} else if (ch === ")") {
round -= 1;
} else if (ch === "[") {
square += 1;
} else if (ch === "]") {
square -= 1;
}
}
return { openQuote: Boolean(quote), round, square };
}
// Returns a short reason when the expression is provably incomplete, else null.
export function describeUnfinished(value) {
const text = (value || "").trim();
if (!text) return "is empty";
const { openQuote, round, square } = scan(text);
if (openQuote) return "has a quote that is still open";
if (round > 0) return "has a ( that is never closed";
if (square > 0) return "has a [ that is never closed";
// Unbalanced the other way is a mistake, not an unfinished edit: let the
// processor report it properly.
if (round < 0 || square < 0) return null;
// "/" on its own is the root pattern, not a path someone abandoned midway.
if (text === "/") return null;
for (const token of TRAILING_SYMBOLS) {
if (text.endsWith(token)) return `stops at "${token}"`;
}
const words = text.split(/\s+/);
if (words.length > 1 && TRAILING_WORDS.includes(words[words.length - 1])) {
return `stops at "${words[words.length - 1]}"`;
}
return null;
}
function elementLabel(el) {
return `<${el.prefix ? `${el.prefix}:` : ""}${el.localName}>`;
}
// Above this the check is skipped entirely. It runs on every keystroke, on top
// of the well-formedness parse, and the walk is not free on a large document
// (jsdom: ~2ms at 3KB, ~15ms at 34KB, ~250ms at 100KB). It also stops being
// worth it: nobody types a 64KB stylesheet character by character — a document
// that size was pasted, and a pasted document is finished. So the cost is
// capped where the problem it solves stops existing.
const MAX_SCANNED_BYTES = 64 * 1024;
// Scans a stylesheet for the first expression that is still being typed.
// Returns { message } or null. Callers pass text that is already known to
// parse; anything that does not parse is the well-formedness gate's business.
export function findUnfinishedExpression(xslt) {
if (!xslt || !xslt.trim()) return null;
if (xslt.length > MAX_SCANNED_BYTES) return null;
let doc;
try {
doc = new DOMParser().parseFromString(xslt, "application/xml");
} catch {
return null; // no parser here: let the backend decide, as before
}
if (doc.querySelector("parsererror")) return null;
// Array.from, not for..of: an HTMLCollection is only iterable by grace of
// the browsers, and this has to hold in every one of them.
const elements = Array.from(doc.getElementsByTagNameNS(XSLT_NS, "*"));
for (const el of elements) {
const required = REQUIRED[el.localName];
if (required && !el.hasAttribute(required)) {
// A sequence constructor is a legitimate alternative to value-of/@select.
if (el.localName === "value-of" && el.childNodes.length > 0) continue;
return {
message: `${elementLabel(el)} has no ${required} expression yet`,
};
}
for (const attr of XPATH_ATTRS) {
if (!el.hasAttribute(attr)) continue;
const value = el.getAttribute(attr);
const reason = describeUnfinished(value);
if (reason) {
const shown = value.trim();
return {
message: shown
? `${attr}="${shown}" on ${elementLabel(el)} ${reason}`
: `${attr} on ${elementLabel(el)} ${reason}`,
};
}
}
}
return null;
}
@@ -0,0 +1,160 @@
import { describe, it, expect } from "vitest";
import {
describeUnfinished,
findUnfinishedExpression,
} from "./unfinishedExpression";
import { TEMPLATES, STARTER_STYLESHEET } from "./templates";
const wrap = (body, version = "2.0") =>
`<xsl:stylesheet version="${version}" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">${body}</xsl:stylesheet>`;
describe("describeUnfinished — expressions that are still being typed", () => {
// Every one of these came off a day of production logs.
it.each([
["", "is empty"],
[" ", "is empty"],
["/Shop/Category/", 'stops at "/"'],
["/Shop/Category/@", 'stops at "@"'],
["current()/", 'stops at "/"'],
["../", 'stops at "/"'],
["..//", 'stops at "//"'],
["./@", 'stops at "@"'],
["current()/..[", 'has a [ that is never closed'],
["Book[@CategoryId = ", "has a [ that is never closed"],
["concat(@a,", "has a ( that is never closed"],
["@Name = 'Programm", "has a quote that is still open"],
["$total +", 'stops at "+"'],
["@a =", 'stops at "="'],
["item[@id = $", 'has a [ that is never closed'],
["1 to", 'stops at "to"'],
["@a and", 'stops at "and"'],
])("flags %j", (value, reason) => {
expect(describeUnfinished(value)).toBe(reason);
});
});
describe("describeUnfinished — expressions it must leave alone", () => {
it.each([
"/", // the root pattern, not an abandoned path
".",
"..",
"*",
"@*",
"@id",
"node()",
"text()",
"child::*",
"//item",
"/Shop/Category",
"$total",
"count(item)",
"concat(@a, '-', @b)",
"item[@id = '3']",
"item[position() > 1]",
"current()/..",
"1 to 5",
"@a and @b",
"and", // a path selecting <and> children is legal
"or",
"to",
"@price * 1.21",
"-1",
"@a - 1",
"substring(@a, 1, 2)",
"if ($a) then 'x' else 'y'",
"for $i in item return $i",
"@a != @b",
"'a literal with a ( in it'",
"'unbalanced ] inside a string'",
"item[1]/name",
"xs:date('2026-01-01')",
"map{'a': 1}",
])("stays quiet on %j", (value) => {
expect(describeUnfinished(value)).toBeNull();
});
it("leaves a genuinely wrong expression to the processor", () => {
// Closing more than was opened is a mistake, not a half-finished edit: the
// real error message is more useful than us guessing.
expect(describeUnfinished("item)")).toBeNull();
expect(describeUnfinished("item]")).toBeNull();
});
});
describe("findUnfinishedExpression", () => {
it("catches the attribute that is not there yet", () => {
// "Required attribute 'select' is missing" was the single most common
// error in production: 65 hits on one line in 24 hours.
expect(
findUnfinishedExpression(
wrap(`<xsl:template match="/"><xsl:value-of/></xsl:template>`),
).message,
).toBe("<xsl:value-of> has no select expression yet");
expect(
findUnfinishedExpression(
wrap(`<xsl:template match="/"><xsl:if>x</xsl:if></xsl:template>`),
).message,
).toBe("<xsl:if> has no test expression yet");
});
it("allows a sequence constructor in place of value-of/@select", () => {
expect(
findUnfinishedExpression(
wrap(
`<xsl:template match="/"><xsl:value-of><xsl:sequence select="1"/></xsl:value-of></xsl:template>`,
),
),
).toBeNull();
});
it("names the attribute and the element it is on", () => {
const found = findUnfinishedExpression(
wrap(`<xsl:template match="/"><xsl:for-each select="/Shop/Category/"/></xsl:template>`),
);
expect(found.message).toBe(
'select="/Shop/Category/" on <xsl:for-each> stops at "/"',
);
});
it("ignores attributes that are not expressions", () => {
expect(
findUnfinishedExpression(
wrap(`<xsl:template match="/"><xsl:element name="a-"/></xsl:template>`),
),
).toBeNull();
});
it("says nothing about a document that does not parse", () => {
// That is the well-formedness gate's job, and it reports it better.
expect(findUnfinishedExpression(`<xsl:stylesheet><xsl:template`)).toBeNull();
});
it("says nothing about an empty editor", () => {
expect(findUnfinishedExpression("")).toBeNull();
expect(findUnfinishedExpression(" ")).toBeNull();
});
it("passes every stylesheet the app itself ships", () => {
// The strongest guard against a false positive: if the gate would hold back
// the starter document or any gallery template, it is wrong.
expect(findUnfinishedExpression(STARTER_STYLESHEET)).toBeNull();
for (const template of TEMPLATES) {
expect(
findUnfinishedExpression(template.xslt),
`template ${template.id} must run`,
).toBeNull();
}
});
});
describe("cost", () => {
it("skips a stylesheet too large to have been typed", () => {
const filler = "<xsl:comment>x</xsl:comment>".repeat(3000);
const big = wrap(
`${filler}<xsl:template match="/"><xsl:value-of select="/a/"/></xsl:template>`,
);
expect(big.length).toBeGreaterThan(64 * 1024);
expect(findUnfinishedExpression(big)).toBeNull();
});
});