Extract retry logging into helper method and fix formatting

- Add LogRetryAttempt() helper method for consistent logging
- Fix formatting in ShouldRetryRequest() for better readability
- Use helper method in both GetRecentTracksRequestFinished and GetTopTracksRequestFinished
- Eliminates duplicate logging code

Co-authored-by: jonaski <10343810+jonaski@users.noreply.github.com>

src/constants/scrobblersettings.h

@@ -34,6 +34,7 @@ constexpr char kShowErrorDialog[] = "show_error_dialog";
 constexpr char kStripRemastered[] = "strip_remastered";
 constexpr char kSources[] = "sources";
 constexpr char kUserToken[] = "user_token";
+constexpr char kApiKey[] = "api_key";
 
 }  // namespace ScrobblerSettings
 

src/scrobbler/lastfmimport.cpp

@@ -42,15 +42,22 @@
 #include "core/logging.h"
 #include "core/networkaccessmanager.h"
 #include "core/settings.h"
+#include "constants/scrobblersettings.h"
 
 #include "lastfmimport.h"
 
 #include "lastfmscrobbler.h"
 
 using namespace Qt::Literals::StringLiterals;
+using namespace ScrobblerSettings;
 
 namespace {
 constexpr int kRequestsDelay = 2000;
+constexpr int kMaxRetries = 5;
+constexpr int kInitialBackoffMs = 5000;
+constexpr int kMaxBackoffShift = 10;  // Maximum shift value to prevent overflow
+constexpr int kRetryHttpStatusCode1 = 500;  // Internal Server Error
+constexpr int kRetryHttpStatusCode2 = 503;  // Service Unavailable
 }
 
 LastFMImport::LastFMImport(const SharedPtr<NetworkAccessManager> network, QObject *parent)
@@ -101,14 +108,17 @@ void LastFMImport::ReloadSettings() {
   Settings s;
   s.beginGroup(LastFMScrobbler::kSettingsGroup);
   username_ = s.value("username").toString();
+  api_key_ = s.value(ScrobblerSettings::kApiKey).toString();
   s.endGroup();
 
 }
 
 QNetworkReply *LastFMImport::CreateRequest(const ParamList &request_params) {
 
+  const QString api_key = !api_key_.isEmpty() ? api_key_ : QLatin1String(LastFMScrobbler::kApiKey);
+
   ParamList params = ParamList()
-    << Param(u"api_key"_s, QLatin1String(LastFMScrobbler::kApiKey))
+    << Param(u"api_key"_s, api_key)
     << Param(u"user"_s, username_)
     << Param(u"lang"_s, QLocale().name().left(2).toLower())
     << Param(u"format"_s, u"json"_s)
@@ -234,11 +244,11 @@ void LastFMImport::SendGetRecentTracksRequest(GetRecentTracksRequest request) {
   }
 
   QNetworkReply *reply = CreateRequest(params);
-  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetRecentTracksRequestFinished(reply, request.page); });
+  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetRecentTracksRequestFinished(reply, request); });
 
 }
 
-void LastFMImport::GetRecentTracksRequestFinished(QNetworkReply *reply, const int page) {
+void LastFMImport::GetRecentTracksRequestFinished(QNetworkReply *reply, GetRecentTracksRequest request) {
 
   if (!replies_.contains(reply)) return;
   replies_.removeAll(reply);
@@ -247,10 +257,21 @@ void LastFMImport::GetRecentTracksRequestFinished(QNetworkReply *reply, const in
 
   const JsonObjectResult json_object_result = ParseJsonObject(reply);
   if (!json_object_result.success()) {
+    if (ShouldRetryRequest(json_object_result) && request.retry_count < kMaxRetries) {
+      const int delay_ms = CalculateBackoffDelay(request.retry_count);
+      LogRetryAttempt(json_object_result.http_status_code, request.retry_count, delay_ms);
+      QTimer::singleShot(delay_ms, this, [this, request]() {
+        GetRecentTracksRequest retry_request(request.page, request.retry_count + 1);
+        SendGetRecentTracksRequest(retry_request);
+      });
+      return;
+    }
     Error(json_object_result.error_message);
     return;
   }
 
+  const int page = request.page;
+
   QJsonObject json_object = json_object_result.json_object;
   if (json_object.isEmpty()) {
     return;
@@ -390,11 +411,11 @@ void LastFMImport::SendGetTopTracksRequest(GetTopTracksRequest request) {
   }
 
   QNetworkReply *reply = CreateRequest(params);
-  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetTopTracksRequestFinished(reply, request.page); });
+  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetTopTracksRequestFinished(reply, request); });
 
 }
 
-void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, const int page) {
+void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, GetTopTracksRequest request) {
 
   if (!replies_.contains(reply)) return;
   replies_.removeAll(reply);
@@ -403,10 +424,21 @@ void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, const int p
 
   const JsonObjectResult json_object_result = ParseJsonObject(reply);
   if (!json_object_result.success()) {
+    if (ShouldRetryRequest(json_object_result) && request.retry_count < kMaxRetries) {
+      const int delay_ms = CalculateBackoffDelay(request.retry_count);
+      LogRetryAttempt(json_object_result.http_status_code, request.retry_count, delay_ms);
+      QTimer::singleShot(delay_ms, this, [this, request]() {
+        GetTopTracksRequest retry_request(request.page, request.retry_count + 1);
+        SendGetTopTracksRequest(retry_request);
+      });
+      return;
+    }
     Error(json_object_result.error_message);
     return;
   }
 
+  const int page = request.page;
+
   QJsonObject json_object = json_object_result.json_object;
   if (json_object.isEmpty()) {
     return;
@@ -516,6 +548,23 @@ void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, const int p
 
 }
 
+bool LastFMImport::ShouldRetryRequest(const JsonObjectResult &result) const {
+  return result.http_status_code == kRetryHttpStatusCode1 ||
+         result.http_status_code == kRetryHttpStatusCode2 ||
+         result.network_error == QNetworkReply::TemporaryNetworkFailureError;
+}
+
+void LastFMImport::LogRetryAttempt(const int http_status_code, const int retry_count, const int delay_ms) const {
+  qLog(Warning) << "Last.fm request failed with status" << http_status_code
+                << ", retrying in" << delay_ms << "ms (attempt"
+                << (retry_count + 1) << "of" << kMaxRetries << ")";
+}
+
+int LastFMImport::CalculateBackoffDelay(const int retry_count) const {
+  const int safe_shift = std::min(retry_count, kMaxBackoffShift);
+  return kInitialBackoffMs * (1 << safe_shift);
+}
+
 void LastFMImport::UpdateTotalCheck() {
 
   Q_EMIT UpdateTotal(lastplayed_total_, playcount_total_);

src/scrobbler/lastfmimport.h

@@ -60,12 +60,14 @@ class LastFMImport : public JsonBaseRequest {
   using ParamList = QList<Param>;
 
   struct GetRecentTracksRequest {
-    explicit GetRecentTracksRequest(const int _page) : page(_page) {}
+    explicit GetRecentTracksRequest(const int _page, const int _retry_count = 0) : page(_page), retry_count(_retry_count) {}
     int page;
+    int retry_count;
   };
   struct GetTopTracksRequest {
-    explicit GetTopTracksRequest(const int _page) : page(_page) {}
+    explicit GetTopTracksRequest(const int _page, const int _retry_count = 0) : page(_page), retry_count(_retry_count) {}
     int page;
+    int retry_count;
   };
 
  private:
@@ -78,6 +80,10 @@ class LastFMImport : public JsonBaseRequest {
   void SendGetRecentTracksRequest(GetRecentTracksRequest request);
   void SendGetTopTracksRequest(GetTopTracksRequest request);
 
+  bool ShouldRetryRequest(const JsonObjectResult &result) const;
+  int CalculateBackoffDelay(const int retry_count) const;
+  void LogRetryAttempt(const int http_status_code, const int retry_count, const int delay_ms) const;
+
   void Error(const QString &error, const QVariant &debug = QVariant()) override;
 
   void UpdateTotalCheck();
@@ -95,14 +101,15 @@ class LastFMImport : public JsonBaseRequest {
 
  private Q_SLOTS:
   void FlushRequests();
-  void GetRecentTracksRequestFinished(QNetworkReply *reply, const int page);
-  void GetTopTracksRequestFinished(QNetworkReply *reply, const int page);
+  void GetRecentTracksRequestFinished(QNetworkReply *reply, GetRecentTracksRequest request);
+  void GetTopTracksRequestFinished(QNetworkReply *reply, GetTopTracksRequest request);
 
  private:
   SharedPtr<NetworkAccessManager> network_;
   QTimer *timer_flush_requests_;
 
   QString username_;
+  QString api_key_;
   bool lastplayed_;
   bool playcount_;
   int playcount_total_;

src/scrobbler/lastfmscrobbler.cpp

@@ -113,6 +113,7 @@ void LastFMScrobbler::ReloadSettings() {
 
   s.beginGroup(kSettingsGroup);
   enabled_ = s.value(ScrobblerSettings::kEnabled, false).toBool();
+  api_key_ = s.value(ScrobblerSettings::kApiKey).toString();
   s.endGroup();
 
   s.beginGroup(ScrobblerSettings::kSettingsGroup);

src/scrobbler/lastfmscrobbler.h

@@ -64,6 +64,7 @@ class LastFMScrobbler : public ScrobblerService {
   bool subscriber() const { return subscriber_; }
   bool submitted() const override { return submitted_; }
   QString username() const { return username_; }
+  QString api_key() const { return api_key_; }
 
   void Authenticate();
   void UpdateNowPlaying(const Song &song) override;
@@ -139,6 +140,7 @@ class LastFMScrobbler : public ScrobblerService {
   bool subscriber_;
   QString username_;
   QString session_key_;
+  QString api_key_;
 
   bool submitted_;
   Song song_playing_;

src/settings/scrobblersettingspage.cpp

@@ -106,6 +106,7 @@ void ScrobblerSettingsPage::Load() {
   ui_->checkbox_source_unknown->setChecked(scrobbler_->sources().contains(Song::Source::Unknown));
 
   ui_->checkbox_lastfm_enable->setChecked(lastfmscrobbler_->enabled());
+  ui_->lineedit_lastfm_api_key->setText(lastfmscrobbler_->api_key());
   LastFM_RefreshControls(lastfmscrobbler_->authenticated());
 
   ui_->checkbox_listenbrainz_enable->setChecked(listenbrainzscrobbler_->enabled());
@@ -152,6 +153,7 @@ void ScrobblerSettingsPage::Save() {
 
   s.beginGroup(LastFMScrobbler::kSettingsGroup);
   s.setValue(kEnabled, ui_->checkbox_lastfm_enable->isChecked());
+  s.setValue(kApiKey, ui_->lineedit_lastfm_api_key->text());
   s.endGroup();
 
   s.beginGroup(ListenBrainzScrobbler::kSettingsGroup);

src/settings/scrobblersettingspage.ui

@@ -234,6 +234,43 @@
         </property>
        </widget>
       </item>
+      <item>
+       <layout class="QHBoxLayout" name="layout_lastfm_api_key">
+        <item>
+         <widget class="QLabel" name="label_lastfm_api_key">
+          <property name="minimumSize">
+           <size>
+            <width>80</width>
+            <height>0</height>
+           </size>
+          </property>
+          <property name="text">
+           <string>API key:</string>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <widget class="QLineEdit" name="lineedit_lastfm_api_key">
+          <property name="placeholderText">
+           <string>Optional - your own Last.fm API key</string>
+          </property>
+         </widget>
+        </item>
+       </layout>
+      </item>
+      <item>
+       <widget class="QLabel" name="label_lastfm_api_key_info">
+        <property name="text">
+         <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot; font-size:8pt;&quot;&gt;Using your own API key can help avoid rate limiting for large libraries. Get one at &lt;/span&gt;&lt;a href=&quot;https://www.last.fm/api/account/create&quot;&gt;&lt;span style=&quot; font-size:8pt; text-decoration: underline; color:#0000ff;&quot;&gt;https://www.last.fm/api/account/create&lt;/span&gt;&lt;/a&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
+        </property>
+        <property name="wordWrap">
+         <bool>true</bool>
+        </property>
+        <property name="openExternalLinks">
+         <bool>true</bool>
+        </property>
+       </widget>
+      </item>
       <item>
        <widget class="LoginStateWidget" name="widget_lastfm_login_state" native="true"/>
       </item>
@@ -394,6 +431,7 @@
   <tabstop>checkbox_source_somafm</tabstop>
   <tabstop>checkbox_source_radioparadise</tabstop>
   <tabstop>checkbox_lastfm_enable</tabstop>
+  <tabstop>lineedit_lastfm_api_key</tabstop>
   <tabstop>button_lastfm_login</tabstop>
   <tabstop>checkbox_listenbrainz_enable</tabstop>
   <tabstop>lineedit_listenbrainz_user_token</tabstop>