Skip to content

Commit

Permalink
calling store.UpdateFeed and store.UpdateFeedError in a defer function
Browse files Browse the repository at this point in the history
This makes sure that we are always calling the following functions:
* originalFeed.ScheduleNextCheck
* store.UpdateFeed
* store.UpdateFeedError
And we only call them in a single place regardless which branch is taken inside function.

The store functions within them will udpate the value in database.

This change aims to reduce "unknown unknowns" in the codes. With this change, we won't miss calling above functions.
  • Loading branch information
shizunge committed Oct 7, 2024
1 parent ee8c662 commit 2231ec2
Showing 1 changed file with 33 additions and 36 deletions.
69 changes: 33 additions & 36 deletions internal/reader/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,13 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
}

// RefreshFeed refreshes a feed.
func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool) *locale.LocalizedErrorWrapper {
func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool) (localizedError *locale.LocalizedErrorWrapper) {
slog.Debug("Begin feed refresh process",
slog.Int64("user_id", userID),
slog.Int64("feed_id", feedID),
slog.Bool("force_refresh", forceRefresh),
)
localizedError = nil

user, storeErr := store.UserByID(userID)
if storeErr != nil {
Expand All @@ -220,7 +221,31 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
}

originalFeed.CheckedNow()
originalFeed.ScheduleNextCheck(weeklyEntryCount, refreshDelayInMinutes)
// Commit the result to the database at the end of this function.
// If we met an error before entering the defer function, localizedError would not be nil.
defer func() {
originalFeed.ScheduleNextCheck(weeklyEntryCount, refreshDelayInMinutes)
slog.Debug("Updated next check date",
slog.Int64("user_id", userID),
slog.Int64("feed_id", feedID),
slog.Int("weeklyEntryCount", weeklyEntryCount),
slog.Int("retry_delay_in_seconds", retryDelayInSeconds),

Check failure on line 232 in internal/reader/handler/handler.go

View workflow job for this annotation

GitHub Actions / Integration Tests

undefined: retryDelayInSeconds

Check failure on line 232 in internal/reader/handler/handler.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

undefined: retryDelayInSeconds

Check failure on line 232 in internal/reader/handler/handler.go

View workflow job for this annotation

GitHub Actions / Golang Linters

undefined: retryDelayInSeconds

Check failure on line 232 in internal/reader/handler/handler.go

View workflow job for this annotation

GitHub Actions / Unit Tests (ubuntu-latest, 1.23.x)

undefined: retryDelayInSeconds

Check failure on line 232 in internal/reader/handler/handler.go

View workflow job for this annotation

GitHub Actions / Analyze (javascript)

undefined: retryDelayInSeconds

Check failure on line 232 in internal/reader/handler/handler.go

View workflow job for this annotation

GitHub Actions / Unit Tests (macOS-latest, 1.23.x)

undefined: retryDelayInSeconds
slog.Int("refresh_delay_in_minutes", refreshDelayInMinutes),
slog.Time("new_next_check_at", originalFeed.NextCheckAt),
)
if localizedError == nil {
// We have not encountered any error before entering this delay function.
originalFeed.ResetErrorCounter()
if storeErr := store.UpdateFeed(originalFeed); storeErr != nil {
// Update the return value when there is an error.
localizedError = locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
}
}
if localizedError != nil {
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
}
}()

requestBuilder := fetcher.NewRequestBuilder()
requestBuilder.WithUsernameAndPassword(originalFeed.Username, originalFeed.Password)
Expand Down Expand Up @@ -254,17 +279,13 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
)
}

if localizedError := responseHandler.LocalizedError(); localizedError != nil {
if localizedError = responseHandler.LocalizedError(); localizedError != nil {
slog.Warn("Unable to fetch feed", slog.String("feed_url", originalFeed.FeedURL), slog.Any("error", localizedError.Error()))
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
}

if store.AnotherFeedURLExists(userID, originalFeed.ID, responseHandler.EffectiveURL()) {
localizedError := locale.NewLocalizedErrorWrapper(ErrDuplicatedFeed, "error.duplicated_feed")
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
localizedError = locale.NewLocalizedErrorWrapper(ErrDuplicatedFeed, "error.duplicated_feed")
return localizedError
}

Expand All @@ -284,40 +305,25 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool

updatedFeed, parseErr := parser.ParseFeed(responseHandler.EffectiveURL(), bytes.NewReader(responseBody))
if parseErr != nil {
localizedError := locale.NewLocalizedErrorWrapper(parseErr, "error.unable_to_parse_feed", parseErr)

if errors.Is(parseErr, parser.ErrFeedFormatNotDetected) {
localizedError = locale.NewLocalizedErrorWrapper(parseErr, "error.feed_format_not_detected", parseErr)
} else {
localizedError = locale.NewLocalizedErrorWrapper(parseErr, "error.unable_to_parse_feed", parseErr)
}

originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
}

// If the feed has a TTL defined, we use it to make sure we don't check it too often.
refreshDelayInMinutes = updatedFeed.TTL

// Set the next check at with updated arguments.
originalFeed.ScheduleNextCheck(weeklyEntryCount, refreshDelayInMinutes)

slog.Debug("Updated next check date",
slog.Int64("user_id", userID),
slog.Int64("feed_id", feedID),
slog.Int("refresh_delay_in_minutes", refreshDelayInMinutes),
slog.Time("new_next_check_at", originalFeed.NextCheckAt),
)

originalFeed.Entries = updatedFeed.Entries
processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh)

// We don't update existing entries when the crawler is enabled (we crawl only inexisting entries). Unless it is forced to refresh
updateExistingEntries := forceRefresh || !originalFeed.Crawler
newEntries, storeErr := store.RefreshFeedEntries(originalFeed.UserID, originalFeed.ID, originalFeed.Entries, updateExistingEntries)
if storeErr != nil {
localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
localizedError = locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
return localizedError
}

Expand Down Expand Up @@ -354,14 +360,5 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
}
}

originalFeed.ResetErrorCounter()

if storeErr := store.UpdateFeed(originalFeed); storeErr != nil {
localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
store.UpdateFeedError(originalFeed)
return localizedError
}

return nil
return localizedError
}

0 comments on commit 2231ec2

Please sign in to comment.