Handle an icon file that exists but isn't valid SVG - #19
Open
pcbeingused333 wants to merge 2 commits into
Open
pcbeingused333 wants to merge 2 commits into
pcbeingused333 wants to merge 2 commits into
Conversation
`Sprite#symbol_from` reads the icon file and calls `["viewBox"]` on
`Nokogiri::XML(...).at_css("svg")`. When the file exists but isn't valid
SVG (empty, truncated, or HTML), `at_css("svg")` returns `nil` and the
whole sprite build dies with `NoMethodError: undefined method '[]' for
nil`. A missing icon is already handled by warning and skipping; an
unparseable one now is too.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
`Icon#svg` has the same `at_css("svg")` -> `nil` path as the sprite
builder: a file that exists but isn't valid SVG passed `nil` into
`attach_attributes`, raising `NoMethodError`. It now raises
`Icons::IconNotFound` with a message noting the file is not valid SVG,
alongside the existing `Errno::ENOENT` rescue.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GtYgwYNfJ3wHrw9xrooVoB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three code paths read an icon file and call
at_css("svg")without checking the result. When the file exists but isn't valid SVG — empty, truncated, or an HTML error page from a bad sync —at_css("svg")returnsniland the caller crashes withNoMethodError.Sprite#symbol_fromA missing icon is already handled (
rescue Icons::IconNotFound→ warn → skip). An unparseable one now is too — warn and skip.Icon#svgNow raises
Icons::IconNotFound(with a message noting the file isn't valid SVG), next to the existingErrno::ENOENTrescue.SpriteIcon#sprite_svgparses a fixed template, so it's fine.Tests
sprite_test.rb— a temp library with one junk.svgand one real one: the sprite keeps the real symbol, omits the junk one, warns.icon_test.rb— an empty.svgraisesIconNotFound.Both fail on
main(NoMethodError).bundle exec rakeis green (91 runs, 0 failures).🤖 Generated with Claude Code