Skip to content

feat(svg): every reader error names the offending element and why#184

Merged
DemchaAV merged 2 commits into
developfrom
feat/svg-error-context
Jun 13, 2026
Merged

feat(svg): every reader error names the offending element and why#184
DemchaAV merged 2 commits into
developfrom
feat/svg-error-context

Conversation

@DemchaAV

Copy link
Copy Markdown
Owner

Second of the two requested follow-ups: when you accept a real-world SVG and something won't render, the error now explains itself — which element, which attribute, and how to fix it — instead of a cryptic one-liner.

Before → after

unsupported SVG colour 'rebecca-purple'
  →  in <circle cx="12" cy="12" fill="rebecca-purple" r="10">: unsupported SVG colour
     'rebecca-purple' — use #hex (3/4/6/8 digits), rgb()/rgba(), a CSS colour name, none, or currentColor

paint 'url(#missing)' references no <linearGradient>...
  →  in <path d="M0 0 H24" stroke="url(#missing)" stroke-width="2">: paint 'url(#missing)' references no
     <linearGradient>/<radialGradient> with id 'missing'

SVG document contains no drawable geometry
  →  SVG document contains no drawable geometry — skipped text; this reader renders vector shapes only
     (no text, images, <use>, masks, clips or filters)

How

  • The reader wraps each element's own processing in one try that prepends a compact, log-safe descriptor (<tag attr="…">, long d truncated). Recursion stays outside the try, so a nested error pinpoints the deepest failing element, never its wrapping <g>, and never double-wraps (pinned by a test).
  • The empty-icon error names the dropped element kinds, so a text-only / use-only SVG says why it's blank.
  • NumberFormatException (bad coordinate) is a subclass of IllegalArgumentException, so it gets element context too.

Housekeeping

This change pushed SvgIconReader to 514 LOC; I extracted the shape-lowering (rect/ellipse/points → path data) to a focused SvgShapeLowering helper (reader back to 469) with its own 4-test unit — the PR-85 discipline.

Verification

./mvnw verify -pl .BUILD SUCCESS, +8 tests (4 error-context: element-named colour, deepest-not-group, long-attr truncation, blank-explains-skip; 4 shape-lowering). Existing 25 SvgIconTest cases stay green — the wrapping preserves every hasMessageContaining substring. No public API change (messages only). Independent of any open branch.

Pairs with #183 (block alignment) — the two things you asked for.

Accepting a real-world SVG that won't render now explains itself instead of
a cryptic message:
- each per-element error is wrapped with element context — the deepest
  failing element (not its wrapping <g>), its attributes (long values like d
  truncated), then the original reason and the supported set:
  'in <circle fill="rebecca-purple" …>: unsupported SVG colour … — use #hex,
  rgb()/rgba(), a CSS colour name, …'
- a blank result names what was dropped: 'no drawable geometry — skipped
  text; this reader renders vector shapes only', not a bare 'no geometry'
- shape lowering (rect/ellipse/points → path data) extracted to
  SvgShapeLowering (this change crossed 500 LOC; back to 469) with its own
  4-test unit; the reader's per-element try wraps once, recursion stays
  outside so child errors never double-wrap

+8 tests (4 error-context in SvgIconTest, 4 in SvgShapeLoweringTest)
Comment thread src/main/java/com/demcha/compose/document/svg/SvgIconReader.java Fixed
Comment thread src/main/java/com/demcha/compose/document/svg/SvgIconReader.java Fixed
num()/viewBox()/transform parsing routed through a parseNumber helper that
throws '<field> must be a number, got <value>' with the cause chained,
instead of leaking the raw JDK 'For input string'. Drops a dead emitLayer
param and fixes the describe() truncation off-by-one. Adds negative tests.
}

/** Builds and appends the layer for a drawable element (curve geometry + paint). */
private static void emitLayer(Element element, String d, Paint paint,
@DemchaAV DemchaAV merged commit c0cd5ec into develop Jun 13, 2026
11 checks passed
@DemchaAV DemchaAV deleted the feat/svg-error-context branch June 13, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants