Skip to content

fix: Improve error reporting #180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
guyfedwards merged 1 commit into from
Jan 23, 2026
Merged

fix: Improve error reporting #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions cmd/nom/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,13 @@ func main() {
_, err := parser.Parse()
// check for help flag
if err != nil {
if flagErr, ok := err.(*flags.Error); ok && flagErr.Type != flags.ErrHelp {
parser.WriteHelp(os.Stdout)
if flagErr, ok := err.(*flags.Error); ok && flagErr.Type == flags.ErrHelp {
os.Exit(0)
}

os.Exit(0)
// For actual errors, print help and exit with error code
fmt.Fprintf(os.Stderr, "Error: %s\n\n", err)
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When flag parsing fails (non-help), surface the error on stderr, show help, and exit non-zero so CLI failures are visible to users/scripts.

Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason: treat real CLI parse errors as failures (exit 1 + stderr) so scripts can detect them; help requests should still be a clean exit.

parser.WriteHelp(os.Stderr)
os.Exit(1)
}

// no subcommand or help flag, run the TUI
Expand Down
6 changes: 4 additions & 2 deletions internal/commands/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func (c Commands) fetchAllFeeds() ([]store.Item, []ErrorItem, error) {

err := c.store.UpsertItem(&i)
if err != nil {
log.Fatalf("[commands.go] fetchAllFeeds: %e", err)
log.Printf("[commands.go] fetchAllFeeds: failed to upsert item: %v", err)
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid crashing the entire refresh on a single bad item; log the upsert failure and keep processing the rest.

Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid crashing the whole app on a single item upsert; log and continue so other feeds still load.

continue
}

Expand Down Expand Up @@ -381,7 +381,9 @@ func htmlToMd(html string) string {

mdown, err := converter.ConvertString(html)
if err != nil {
log.Fatal(err)
log.Printf("[commands.go] htmlToMd: failed to convert HTML to markdown: %v", err)
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HTML→Markdown conversion fails, log and fall back to raw HTML so the entry still renders instead of killing the app.

// Return the original HTML if conversion fails
return html
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HTML->MD conversion fails, fall back to original HTML instead of exiting so users still see content.

}

return mdown
Expand Down
10 changes: 5 additions & 5 deletions internal/commands/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (d itemDelegate) Render(w io.Writer, m list.Model, index int, listItem list
func (m *model) UpdateList() tea.Cmd {
fs, err := m.commands.GetAllFeeds()
if err != nil {
return tea.Quit
return m.list.NewStatusMessage(fmt.Sprintf("Error: %s", err))
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Show a status message instead of quitting so users see why feeds failed to load.

Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tea.Quit with a visible status message to avoid silent exits and keep the TUI responsive.

}

cmd := m.list.SetItems(convertItems(fs))
Expand Down Expand Up @@ -211,7 +211,7 @@ func updateList(msg tea.Msg, m model) (tea.Model, tea.Cmd) {
current := item.(TUIItem)
err := m.commands.store.ToggleRead(current.ID)
if err != nil {
return m, tea.Quit
return m, m.list.NewStatusMessage(fmt.Sprintf("Error marking read: %s", err))
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the TUI alive on toggle-read errors; report the problem in the status area.

}

cmds = append(cmds, m.UpdateList())
Expand Down Expand Up @@ -249,7 +249,7 @@ func updateList(msg tea.Msg, m model) (tea.Model, tea.Cmd) {
current := item.(TUIItem)
err := m.commands.store.ToggleFavourite(current.ID)
if err != nil {
return m, tea.Quit
return m, m.list.NewStatusMessage(fmt.Sprintf("Error toggling favourite: %s", err))
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same rationale as toggle-read: show an error message instead of exiting the session.

}

cmds = append(cmds, m.UpdateList())
Expand Down Expand Up @@ -308,8 +308,8 @@ func updateList(msg tea.Msg, m model) (tea.Model, tea.Cmd) {

content, err := m.commands.GetGlamourisedArticle(*m.selectedArticle)
if err != nil {
// LKS: there should be an error message here
return m, tea.Quit
m.selectedArticle = nil
return m, m.list.NewStatusMessage(fmt.Sprintf("Error opening article: %s", err))
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If rendering fails, clear selection and display an error to avoid leaving the UI stuck in the article view.

}

m.viewport.SetContent(content)
Expand Down
3 changes: 2 additions & 1 deletion internal/commands/tui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ func (m model) OpenLink(url string) tea.Cmd {
for _, o := range m.cfg.Openers {
match, err := regexp.MatchString(o.Regex, url)
if err != nil {
return tea.Quit
log.Printf("[tui.go] OpenLink: invalid regex pattern: %v", err)
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip invalid opener regex patterns with a log so a single bad config entry doesn't terminate the UI.

Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid opener regex shouldn't kill the UI; log and skip this opener instead.

continue
}

if match {
Expand Down
24 changes: 16 additions & 8 deletions internal/commands/viewport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ func updateViewport(msg tea.Msg, m model) (tea.Model, tea.Cmd) {
case key.Matches(msg, ViewportKeyMap.OpenInBrowser):
current, err := m.commands.store.GetItemByID(*m.selectedArticle)
if err != nil {
return m, nil
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error: failed to get article")
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the selected item can't be fetched, reset selection and show an error so the user can continue.

Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the selected article can't be loaded, clear selection and surface an error instead of silently failing.

}

it := ItemToTUIItem(current)
Expand All @@ -52,11 +53,13 @@ func updateViewport(msg tea.Msg, m model) (tea.Model, tea.Cmd) {
case key.Matches(msg, ViewportKeyMap.Favourite):
current, err := m.commands.store.GetItemByID(*m.selectedArticle)
if err != nil {
return m, nil
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error: failed to get article")
}
err = m.commands.store.ToggleFavourite(current.ID)
if err != nil {
return m, tea.Quit
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error toggling favourite")
}

case key.Matches(msg, ViewportKeyMap.Read):
Expand All @@ -65,11 +68,13 @@ func updateViewport(msg tea.Msg, m model) (tea.Model, tea.Cmd) {
}
current, err := m.commands.store.GetItemByID(*m.selectedArticle)
if err != nil {
return m, nil
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error: failed to get article")
}
err = m.commands.store.ToggleRead(current.ID)
if err != nil {
return m, tea.Quit
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error marking read")
}

if !m.commands.config.ShowRead {
Expand All @@ -93,7 +98,8 @@ func updateViewport(msg tea.Msg, m model) (tea.Model, tea.Cmd) {
// trigger refresh to update read indication
content, err := m.commands.GetGlamourisedArticle(*m.selectedArticle)
if err != nil {
return m, tea.Quit
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error rendering article")
Copy link
Copy Markdown
Contributor Author

@majiayu000 majiayu000 Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Render failures now surface as status messages instead of quitting, improving error visibility and resilience.

}
m.viewport.SetContent(content)

Expand All @@ -111,7 +117,8 @@ func updateViewport(msg tea.Msg, m model) (tea.Model, tea.Cmd) {

content, err := m.commands.GetGlamourisedArticle(*m.selectedArticle)
if err != nil {
return m, tea.Quit
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error rendering article")
}

m.viewport.SetContent(content)
Expand All @@ -134,7 +141,8 @@ func updateViewport(msg tea.Msg, m model) (tea.Model, tea.Cmd) {

content, err := m.commands.GetGlamourisedArticle(*m.selectedArticle)
if err != nil {
return m, tea.Quit
m.selectedArticle = nil
return m, m.list.NewStatusMessage("Error rendering article")
}

m.viewport.SetContent(content)
Expand Down