diff --git a/.changes/account-link-errors.md b/.changes/account-link-errors.md new file mode 100644 index 00000000..0d8e52cd --- /dev/null +++ b/.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". diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 77ba657e..afade540 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -74,6 +74,11 @@ const _accountLinkErrorMessages: Record = { 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', }; diff --git a/include/AccountLinkResultCode.h b/include/AccountLinkResultCode.h index eca85ae4..91d22f42 100644 --- a/include/AccountLinkResultCode.h +++ b/include/AccountLinkResultCode.h @@ -11,5 +11,11 @@ namespace OpenShock { 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 diff --git a/src/GatewayConnectionManager.cpp b/src/GatewayConnectionManager.cpp index 270fd6ea..e6640378 100644 --- a/src/GatewayConnectionManager.cpp +++ b/src/GatewayConnectionManager.cpp @@ -127,39 +127,50 @@ AccountLinkResultCode GatewayConnectionManager::Link(std::string_view linkCode) 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(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(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); diff --git a/src/captiveportal/CaptivePortalInstance.cpp b/src/captiveportal/CaptivePortalInstance.cpp index ffed2400..d6e3a390 100644 --- a/src/captiveportal/CaptivePortalInstance.cpp +++ b/src/captiveportal/CaptivePortalInstance.cpp @@ -206,6 +206,21 @@ CaptivePortal::CaptivePortalInstance::CaptivePortalInstance() 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;