Skip to content

fix: recover panics inside the article import instead of crashing the server - #54

Merged
flthibaud merged 2 commits into
developfrom
fix/import-panic-recover
Nov 11, 2026
Merged

flthibaud merged 2 commits into
developfrom
fix/import-panic-recover

Conversation

@flthibaud

Copy link
Copy Markdown
Owner

The import started after a subscription ran ImportArticlesForSubscribers on a goroutine with no recover(), so a panic in the HTML parsing took the whole process down. Only the scheduler path was protected.

  • ImportArticlesForSubscribers now recovers the panic itself, logs it, marks the feed failed through the existing markFailed, and returns it as an error. Both callers are covered from one place.
  • processFeed no longer carries its own recover.
  • New test TestImportArticlesForSubscribersRecoversPanic (no database needed).

Tested with go build ./... && go vet ./... && go test -race ./internal/service ./cmd/api and staticcheck.

Closes #19

@flthibaud

flthibaud commented Nov 11, 2026 •

Copy link
Copy Markdown
Owner Author

Review: OK to merge.

  • Checked: the recover sits above the timeout context, markFailed uses a detached context, both callers (processFeed, importAsync) go through it, and the acceptance points of A panic during the post-subscribe article import crashes the server #19 are met.
  • Build, vet, staticcheck and go test -race ./internal/service ./cmd/api pass locally. The full suite, internal/data DB tests included, was then run against a throwaway postgres:16 with the migrations applied: all pass, none skipped.
  • Minor, not blocking: the recovered panic is logged once as an error here and once more as "feed sync failed" by the caller. The recover also assumes a non-nil logger, which is true everywhere outside tests.

@flthibaud
flthibaud merged commit f02a931 into develop Nov 11, 2026
5 checks passed
@flthibaud
flthibaud deleted the fix/import-panic-recover branch November 11, 2026 23:06
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.

A panic during the post-subscribe article import crashes the server

1 participant