feat: better account linking errors

.changes/account-link-errors.md

@@ -0,0 +1,9 @@
+---
+kind: changed
+---
+Report specific account linking failure reasons instead of a generic internal error
+
+## Release Note
+More descriptive account linking errors
+
+Account linking now reports the specific reason it failed (request failed, timed out, server error, invalid response, or failed to save the token) in both the captive portal and the device logs, instead of a generic "Internal error".

frontend/src/lib/api.ts

@@ -74,6 +74,11 @@ const _accountLinkErrorMessages: Record<string, string> = {
   NoInternetConnection: 'No internet connection',
   InvalidCode: 'Invalid code',
   RateLimited: 'Too many requests',
+  RequestFailed: 'Could not reach the server (check DNS/TLS/connection)',
+  RequestTimedOut: 'Request to the server timed out',
+  ServerError: 'Server returned an unexpected response',
+  InvalidResponse: 'Server sent an invalid response',
+  ConfigSaveFailed: 'Failed to save the auth token to the device',
   InternalError: 'Internal error',
 };
 

include/AccountLinkResultCode.h

@@ -1,9 +1,9 @@
 #pragma once
 
 #include <cstdint>
 
 namespace OpenShock {
   enum class AccountLinkResultCode : uint8_t {
     Success              = 0,
     CodeRequired         = 1,
     InvalidCodeLength    = 2,
@@ -11,5 +11,11 @@
     InvalidCode          = 4,
     RateLimited          = 5,
     InternalError        = 6,
+    // More descriptive failure causes (previously all reported as InternalError)
+    RequestFailed    = 7,   // Could not start/complete the HTTP request (DNS, TLS, connection refused, ...)
+    RequestTimedOut  = 8,   // The request to the backend timed out
+    ServerError      = 9,   // The backend returned an unexpected response code
+    InvalidResponse  = 10,  // The backend response could not be parsed or was missing the auth token
+    ConfigSaveFailed = 11,  // Failed to persist the auth token to flash
   };
 }  // namespace OpenShock

src/GatewayConnectionManager.cpp

@@ -40,16 +40,16 @@
 
 const uint8_t LINK_CODE_LENGTH = 6;
 
 static std::atomic<uint8_t> s_flags                 = 0;
 static std::atomic<int64_t> s_lastAuthFailure       = 0;
 static std::atomic<int64_t> s_lastConnectionAttempt = 0;
 static std::atomic_flag s_isInitializing            = ATOMIC_FLAG_INIT;
 static OpenShock::SimpleMutex s_clientMutex;
 static std::shared_ptr<OpenShock::GatewayClient> s_wsClient = nullptr;
 
 static std::shared_ptr<OpenShock::GatewayClient> GetClient()
 {
   OpenShock::ScopedLock lock__(&s_clientMutex);
   return s_wsClient;
 }
 static void CreateClient(const std::string& authToken)
@@ -127,39 +127,50 @@
   OS_LOGD(TAG, "Attempting to link to account using code %.*s", linkCode.length(), linkCode.data());
 
   if (linkCode.length() != LINK_CODE_LENGTH) {
-    OS_LOGE(TAG, "Invalid link code length");
-    return AccountLinkResultCode::InvalidCode;
+    OS_LOGE(TAG, "Invalid link code length: expected %zu, got %zu", static_cast<size_t>(LINK_CODE_LENGTH), linkCode.length());
+    return AccountLinkResultCode::InvalidCodeLength;
   }
 
   auto response = HTTP::JsonAPI::LinkAccount(linkCode);
 
   if (response.code == 404) {
+    OS_LOGW(TAG, "Account link failed: backend rejected the link code as invalid (404)");
     return AccountLinkResultCode::InvalidCode;
   }
 
-  if (response.result == HTTP::RequestResult::RateLimited) {
-    OS_LOGW(TAG, "Account Link request got ratelimited");
-    return AccountLinkResultCode::RateLimited;
-  }
   if (response.result != HTTP::RequestResult::Success) {
-    OS_LOGE(TAG, "Error while getting auth token: %s %d", response.ResultToString(), response.code);
-
-    return AccountLinkResultCode::InternalError;
+    OS_LOGE(TAG, "Account link request failed: %s (result=%hhu, http=%d)", response.ResultToString(), static_cast<uint8_t>(response.result), response.code);
+
+    switch (response.result) {
+      case HTTP::RequestResult::RateLimited:
+        return AccountLinkResultCode::RateLimited;
+      case HTTP::RequestResult::TimedOut:
+        return AccountLinkResultCode::RequestTimedOut;
+      case HTTP::RequestResult::InvalidURL:
+      case HTTP::RequestResult::RequestFailed:
+        return AccountLinkResultCode::RequestFailed;
+      case HTTP::RequestResult::CodeRejected:
+        return AccountLinkResultCode::ServerError;
+      case HTTP::RequestResult::ParseFailed:
+        return AccountLinkResultCode::InvalidResponse;
+      default:  // InternalError (e.g. no backend domain configured, URI truncated), Cancelled, ...
+        return AccountLinkResultCode::InternalError;
+    }
   }
 
   if (response.code != 200) {
-    OS_LOGE(TAG, "Unexpected response code: %d", response.code);
-    return AccountLinkResultCode::InternalError;
+    OS_LOGE(TAG, "Account link failed: unexpected response code %d from backend", response.code);
+    return AccountLinkResultCode::ServerError;
   }
 
   if (response.data.authToken.empty()) {
-    OS_LOGE(TAG, "Received empty auth token");
-    return AccountLinkResultCode::InternalError;
+    OS_LOGE(TAG, "Account link failed: backend returned an empty auth token");
+    return AccountLinkResultCode::InvalidResponse;
   }
 
   if (!Config::SetBackendAuthToken(std::move(response.data.authToken))) {
-    OS_LOGE(TAG, "Failed to save auth token");
-    return AccountLinkResultCode::InternalError;
+    OS_LOGE(TAG, "Account link failed: could not persist auth token to flash");
+    return AccountLinkResultCode::ConfigSaveFailed;
   }
 
   s_flags.fetch_or(FLAG_LINKED, std::memory_order_relaxed);

src/captiveportal/CaptivePortalInstance.cpp

@@ -1,3 +1,3 @@
 #include <freertos/FreeRTOS.h>
 
 #include "captiveportal/CaptivePortalInstance.h"
@@ -206,6 +206,21 @@
         case ResultCode::RateLimited:
           error = "RateLimited";
           break;
+        case ResultCode::RequestFailed:
+          error = "RequestFailed";
+          break;
+        case ResultCode::RequestTimedOut:
+          error = "RequestTimedOut";
+          break;
+        case ResultCode::ServerError:
+          error = "ServerError";
+          break;
+        case ResultCode::InvalidResponse:
+          error = "InvalidResponse";
+          break;
+        case ResultCode::ConfigSaveFailed:
+          error = "ConfigSaveFailed";
+          break;
         default:
           error = "InternalError";
           break;