From 2231ec2a36a7fbc5e43e4744beab4a1cba196c22 Mon Sep 17 00:00:00 2001 From: Shizun Ge Date: Sun, 6 Oct 2024 21:17:58 -0700 Subject: [PATCH] calling store.UpdateFeed and store.UpdateFeedError in a defer function 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. --- internal/reader/handler/handler.go | 69 ++++++++++++++---------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/internal/reader/handler/handler.go b/internal/reader/handler/handler.go index 2e992b67711..490c8daef39 100644 --- a/internal/reader/handler/handler.go +++ b/internal/reader/handler/handler.go @@ -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 { @@ -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), + 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) @@ -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 } @@ -284,30 +305,17 @@ 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) @@ -315,9 +323,7 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool 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 } @@ -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 }