diff --git a/backend/src/main.go b/backend/src/main.go index c95d0af4..a052bbd2 100644 --- a/backend/src/main.go +++ b/backend/src/main.go @@ -173,6 +173,14 @@ func classifyTransformError(msg string) (code, class string) { "is undefined", "format-number picture", "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 ''", 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 { diff --git a/backend/src/main_test.go b/backend/src/main_test.go index 72f8d452..2aafa443 100644 --- a/backend/src/main_test.go +++ b/backend/src/main_test.go @@ -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"}, {"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"}, + // 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. {"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"}, diff --git a/charts/xslt-playground/Chart.yaml b/charts/xslt-playground/Chart.yaml index 2756a475..d740ecbc 100644 --- a/charts/xslt-playground/Chart.yaml +++ b/charts/xslt-playground/Chart.yaml @@ -1,6 +1,6 @@ apiVersion: v2 name: xslt-playground description: Helm chart for xslt-playground frontend and backend -version: 0.4.1 +version: 0.4.2 appVersion: "0.1.0" diff --git a/charts/xslt-playground/templates/backend-deployment.yaml b/charts/xslt-playground/templates/backend-deployment.yaml index 50c8a347..3dd89424 100644 --- a/charts/xslt-playground/templates/backend-deployment.yaml +++ b/charts/xslt-playground/templates/backend-deployment.yaml @@ -42,6 +42,7 @@ spec: mountPath: {{ .Values.firebase.credentialsMountPath }} readOnly: true {{- end }} + resources: {{- toYaml .Values.resources.backend | nindent 12 }} ports: - containerPort: {{ .Values.service.backend.port }} name: http diff --git a/charts/xslt-playground/values.yaml b/charts/xslt-playground/values.yaml index 6b01eecc..788e1147 100644 --- a/charts/xslt-playground/values.yaml +++ b/charts/xslt-playground/values.yaml @@ -33,7 +33,20 @@ ingress: resources: 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: frontend: @@ -43,7 +56,9 @@ hpa: targetCPUUtilizationPercentage: 80 backend: 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 targetCPUUtilizationPercentage: 80 diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index a649823f..1a994b40 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -16,6 +16,7 @@ import { checkWellFormed, } from "./lib/workspaceUtils"; import { templateToWorkspace, findTemplate, STARTER_STYLESHEET } from "./lib/templates"; +import { findUnfinishedExpression } from "./lib/unfinishedExpression"; import { reviewWorkspace } from "./lib/reviewRules"; import { diffLines } from "./lib/diffUtils"; import { encodeCompact, decodeCompact, toSharePayload, fromSharePayload, saveFiddle, loadFiddle } from "./lib/shareLink"; @@ -1776,6 +1777,25 @@ export default function App() { })); 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); diff --git a/frontend/src/App.test.jsx b/frontend/src/App.test.jsx index d7f010ae..76c0aba8 100644 --- a/frontend/src/App.test.jsx +++ b/frontend/src/App.test.jsx @@ -208,6 +208,20 @@ describe("well-formedness gate", () => { await waitFor(() => expect(fetch).toHaveBeenCalled(), { timeout: 3000 }); }, 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( + '', + ); + render(); + 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 () => { seedWorkspace('` 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 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; +} diff --git a/frontend/src/lib/unfinishedExpression.test.js b/frontend/src/lib/unfinishedExpression.test.js new file mode 100644 index 00000000..87f1196f --- /dev/null +++ b/frontend/src/lib/unfinishedExpression.test.js @@ -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") => + `${body}`; + +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 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(``), + ).message, + ).toBe(" has no select expression yet"); + + expect( + findUnfinishedExpression( + wrap(`x`), + ).message, + ).toBe(" has no test expression yet"); + }); + + it("allows a sequence constructor in place of value-of/@select", () => { + expect( + findUnfinishedExpression( + wrap( + ``, + ), + ), + ).toBeNull(); + }); + + it("names the attribute and the element it is on", () => { + const found = findUnfinishedExpression( + wrap(``), + ); + expect(found.message).toBe( + 'select="/Shop/Category/" on stops at "/"', + ); + }); + + it("ignores attributes that are not expressions", () => { + expect( + findUnfinishedExpression( + wrap(``), + ), + ).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(` { + 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 = "x".repeat(3000); + const big = wrap( + `${filler}`, + ); + expect(big.length).toBeGreaterThan(64 * 1024); + expect(findUnfinishedExpression(big)).toBeNull(); + }); +});