Skip to content

caught File.open() exception and print error#40

Closed
doge1024 wants to merge 1 commit into
dblock:masterfrom
doge1024:printerror
Closed

caught File.open() exception and print error#40
doge1024 wants to merge 1 commit into
dblock:masterfrom
doge1024:printerror

Conversation

@doge1024
Copy link
Copy Markdown

@doge1024 doge1024 commented Dec 3, 2020

No description provided.

@dblock
Copy link
Copy Markdown
Owner

dblock commented Dec 3, 2020

What are you seeing now that you wanted to catch this error? What was the old behavior vs. new behavior?

I am not saying we don't want this, but there are things I dislike:

@dangerpr-bot
Copy link
Copy Markdown

2 Warnings
⚠️ [DEPRECATION] check is deprecated. Please use check! instead.
⚠️ Unless you're refactoring existing code or improving documentation, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#40](https://github.com/dblock/fui/pull/40): Caught file.open() exception and print error - [@doge1024](https://github.com/doge1024).

Generated by 🚫 Danger

@dblock
Copy link
Copy Markdown
Owner

dblock commented Apr 12, 2026

What the contributor did: Wrapped process_code and process_xml in begin/rescue to catch any exception when reading/processing a file, print an error message, and call exit -1.

The motivation: Almost certainly the same invalid byte sequence in UTF-8 error from issue #37 — the contributor hit it and wanted to at least print a useful message instead of a raw
backtrace.

Do we want this? No, and we already did it better:

  1. Root cause fixed — PR Fix ArgumentError: invalid byte sequence in UTF-8 for non-UTF-8 files #47 already solves the underlying problem by reading files as binary and transcoding to UTF-8. The error simply no longer occurs.
  2. The implementation has real problems (which dblock called out):
    - Catches all exceptions (rescue => e) — way too broad
    - process_xml has a bug: uses bare rescue (no => e) but then tries to print #{e} — would raise a NameError instead of helping
    - exit -1 deep inside a library method is an anti-pattern; libraries should raise, not exit
    - Silently exits on any file error — no retries, no skip-and-continue

Verdict: Close it — the underlying issue is resolved properly by #47, and this approach was the wrong fix even as a stopgap.

@dblock dblock closed this Apr 12, 2026
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.

3 participants