From f53add53d64ab4cdac7880f9240a7b42cfd41c3f Mon Sep 17 00:00:00 2001 From: Mike Odom <47093906+modom-ofn@users.noreply.github.com> Date: Tue, 2 Dec 2025 18:30:11 -0500 Subject: [PATCH] refactor: simplify httpx helpers and stabilize tests - add request builders and shared retry helper in media auth/user detail calls; handle read/request errors - simplify token validation retry logic - rename httpx tests for naming rules and dedupe repeated literals with shared constants --- auth-portal/providers/httpx.go | 180 ++++++++++++++-------------- auth-portal/providers/httpx_test.go | 41 ++++--- 2 files changed, 114 insertions(+), 107 deletions(-) diff --git a/auth-portal/providers/httpx.go b/auth-portal/providers/httpx.go index ff23c61..6cd1654 100644 --- a/auth-portal/providers/httpx.go +++ b/auth-portal/providers/httpx.go @@ -28,6 +28,64 @@ func snippet(b []byte, n int) string { return s } +func doWithRetry(req *http.Request) (*http.Response, error) { + var resp *http.Response + var err error + for attempt := 0; attempt < 2; attempt++ { + resp, err = httpx.Do(req) + if err != nil { + if ne, ok := err.(net.Error); ok && ne.Timeout() { + time.Sleep(120 * time.Millisecond) + continue + } + if attempt == 0 { + time.Sleep(120 * time.Millisecond) + continue + } + return nil, err + } + if resp.StatusCode >= 500 && resp.StatusCode < 600 && attempt == 0 { + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + time.Sleep(120 * time.Millisecond) + continue + } + break + } + return resp, err +} + +func buildAuthRequest(baseURL, appName, appVersion, clientID string, body map[string]string) (*http.Request, error) { + loginURL := strings.TrimSuffix(baseURL, "/") + "/Users/AuthenticateByName" + payload, err := json.Marshal(body) + if err != nil { + return nil, err + } + req, err := http.NewRequest(http.MethodPost, loginURL, bytes.NewReader(payload)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", mediaAuthHeader(appName, appVersion, clientID)) + req.Header.Set("X-Emby-Client", appName) + req.Header.Set("X-Emby-Client-Version", appVersion) + return req, nil +} + +func buildUserDetailRequest(serverURL, token, userID string) (*http.Request, error) { + u := strings.TrimSuffix(serverURL, "/") + "/Users/" + userID + req, err := http.NewRequest(http.MethodGet, u, nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/json") + if strings.TrimSpace(token) != "" { + req.Header.Set("Authorization", fmt.Sprintf(`MediaBrowser Token="%s"`, token)) + req.Header.Set("X-MediaBrowser-Token", token) + } + return req, nil +} + // mediaAuthResp matches Emby/Jellyfin auth response shape. type mediaAuthResp struct { AccessToken string `json:"AccessToken"` @@ -48,45 +106,23 @@ type mediaUserDetail struct { // mediaAuthAttempt performs a POST to /Users/AuthenticateByName with the given JSON body fields. func mediaAuthAttempt(prefix, baseURL, appName, appVersion, clientID string, body map[string]string) (mediaAuthResp, error) { - loginURL := strings.TrimSuffix(baseURL, "/") + "/Users/AuthenticateByName" - payload, _ := json.Marshal(body) - req, _ := http.NewRequest(http.MethodPost, loginURL, bytes.NewReader(payload)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", mediaAuthHeader(appName, appVersion, clientID)) - req.Header.Set("X-Emby-Client", appName) - req.Header.Set("X-Emby-Client-Version", appVersion) - if Debugf != nil { - Debugf("%s/auth POST %s", prefix, loginURL) + req, err := buildAuthRequest(baseURL, appName, appVersion, clientID, body) + if err != nil { + return mediaAuthResp{}, err } - // one retry on net error or 5xx - var resp *http.Response - var err error - for attempt := 0; attempt < 2; attempt++ { - resp, err = httpx.Do(req) - if err != nil { - if ne, ok := err.(net.Error); ok && ne.Timeout() { - time.Sleep(120 * time.Millisecond) - continue - } - // retry once for any network error - if attempt == 0 { - time.Sleep(120 * time.Millisecond) - continue - } - return mediaAuthResp{}, err - } - if resp.StatusCode >= 500 && resp.StatusCode < 600 && attempt == 0 { - // read+discard to allow connection reuse before retry - io.Copy(io.Discard, resp.Body) - resp.Body.Close() - time.Sleep(120 * time.Millisecond) - continue - } - break + if Debugf != nil { + Debugf("%s/auth POST %s", prefix, req.URL.String()) + } + resp, err := doWithRetry(req) + if err != nil { + return mediaAuthResp{}, err } defer resp.Body.Close() - raw, _ := io.ReadAll(resp.Body) - if resp.StatusCode != 200 { + raw, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return mediaAuthResp{}, readErr + } + if resp.StatusCode != http.StatusOK { if Warnf != nil { Warnf("%s/auth HTTP %d body=%q", prefix, resp.StatusCode, snippet(raw, 200)) } @@ -104,42 +140,22 @@ func mediaAuthAttempt(prefix, baseURL, appName, appVersion, clientID string, bod // mediaGetUserDetail performs a GET to /Users/{id} using either an API key or access token. func mediaGetUserDetail(prefix, serverURL, token, userID string) (mediaUserDetail, error) { - u := strings.TrimSuffix(serverURL, "/") + "/Users/" + userID - req, _ := http.NewRequest(http.MethodGet, u, nil) - req.Header.Set("Accept", "application/json") - if strings.TrimSpace(token) != "" { - req.Header.Set("Authorization", fmt.Sprintf(`MediaBrowser Token="%s"`, token)) - req.Header.Set("X-MediaBrowser-Token", token) + req, err := buildUserDetailRequest(serverURL, token, userID) + if err != nil { + return mediaUserDetail{}, err } if Debugf != nil { - Debugf("%s/user GET %s (token=%v)", prefix, u, token != "") + Debugf("%s/user GET %s (token=%v)", prefix, req.URL.String(), strings.TrimSpace(token) != "") } - // one retry on net error or 5xx - var resp *http.Response - var err error - for attempt := 0; attempt < 2; attempt++ { - resp, err = httpx.Do(req) - if err != nil { - if ne, ok := err.(net.Error); ok && ne.Timeout() { - time.Sleep(120 * time.Millisecond) - continue - } - if attempt == 0 { - time.Sleep(120 * time.Millisecond) - continue - } - return mediaUserDetail{}, err - } - if resp.StatusCode >= 500 && resp.StatusCode < 600 && attempt == 0 { - io.Copy(io.Discard, resp.Body) - resp.Body.Close() - time.Sleep(120 * time.Millisecond) - continue - } - break + resp, err := doWithRetry(req) + if err != nil { + return mediaUserDetail{}, err } defer resp.Body.Close() - raw, _ := io.ReadAll(resp.Body) + raw, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return mediaUserDetail{}, readErr + } if resp.StatusCode != 200 { if Warnf != nil { Warnf("%s/user HTTP %d body=%q", prefix, resp.StatusCode, snippet(raw, 200)) @@ -158,31 +174,15 @@ func mediaGetUserDetail(prefix, serverURL, token, userID string) (mediaUserDetai // mediaTokenStillValid checks whether X-Emby-Token is currently accepted by the server. func mediaTokenStillValid(prefix, serverURL, token string) (bool, error) { - req, _ := http.NewRequest(http.MethodGet, strings.TrimSuffix(serverURL, "/")+"/users/Me", nil) + req, err := http.NewRequest(http.MethodGet, strings.TrimSuffix(serverURL, "/")+"/users/Me", nil) + if err != nil { + return false, err + } req.Header.Set("X-Emby-Token", token) - var resp *http.Response - var err error - for attempt := 0; attempt < 2; attempt++ { - resp, err = httpx.Do(req) - if err != nil { - if ne, ok := err.(net.Error); ok && ne.Timeout() { - time.Sleep(120 * time.Millisecond) - continue - } - if attempt == 0 { - time.Sleep(120 * time.Millisecond) - continue - } - return false, err - } - if resp.StatusCode >= 500 && resp.StatusCode < 600 && attempt == 0 { - io.Copy(io.Discard, resp.Body) - resp.Body.Close() - time.Sleep(120 * time.Millisecond) - continue - } - break + resp, err := doWithRetry(req) + if err != nil { + return false, err } defer resp.Body.Close() return resp.StatusCode == 200, nil diff --git a/auth-portal/providers/httpx_test.go b/auth-portal/providers/httpx_test.go index c2dfdb5..a28da5e 100644 --- a/auth-portal/providers/httpx_test.go +++ b/auth-portal/providers/httpx_test.go @@ -8,12 +8,19 @@ import ( "testing" ) -func TestMediaAuthAttempt_Success(t *testing.T) { +const ( + headerContentType = "Content-Type" + contentTypeJSON = "application/json" + unexpectedErrFmt = "unexpected err: %v" + expectedCallsFmt = "expected 2 calls, got %d" +) + +func TestMediaAuthAttemptSuccess(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/Users/AuthenticateByName" { t.Fatalf("unexpected path: %s", r.URL.Path) } - w.Header().Set("Content-Type", "application/json") + w.Header().Set(headerContentType, contentTypeJSON) json.NewEncoder(w).Encode(map[string]any{ "AccessToken": "tok123", "User": map[string]any{"Id": "u1", "Name": "alice"}, @@ -23,14 +30,14 @@ func TestMediaAuthAttempt_Success(t *testing.T) { out, err := mediaAuthAttempt("jellyfin", ts.URL, "AuthPortal", "2.0.1", "cid", map[string]string{"Username": "a", "Password": "b"}) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf(unexpectedErrFmt, err) } if out.AccessToken != "tok123" || out.User.ID != "u1" || out.User.Name != "alice" { t.Fatalf("unexpected response: %+v", out) } } -func TestMediaAuthAttempt_RetryOn5xx(t *testing.T) { +func TestMediaAuthAttemptRetryOn5xx(t *testing.T) { var calls int32 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := atomic.AddInt32(&calls, 1) @@ -38,7 +45,7 @@ func TestMediaAuthAttempt_RetryOn5xx(t *testing.T) { http.Error(w, "temp", http.StatusServiceUnavailable) return } - w.Header().Set("Content-Type", "application/json") + w.Header().Set(headerContentType, contentTypeJSON) json.NewEncoder(w).Encode(map[string]any{ "AccessToken": "tok456", "User": map[string]any{"Id": "u2", "Name": "bob"}, @@ -48,20 +55,20 @@ func TestMediaAuthAttempt_RetryOn5xx(t *testing.T) { out, err := mediaAuthAttempt("emby", ts.URL, "AuthPortal", "2.0.1", "cid", map[string]string{"Username": "a", "Password": "b"}) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf(unexpectedErrFmt, err) } if out.AccessToken != "tok456" || out.User.ID != "u2" { t.Fatalf("bad: %+v", out) } if atomic.LoadInt32(&calls) != 2 { - t.Fatalf("expected 2 calls, got %d", calls) + t.Fatalf(expectedCallsFmt, calls) } } -func TestMediaAuthAttempt_BadJSON(t *testing.T) { +func TestMediaAuthAttemptBadJSON(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.Write([]byte("not-json")) + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write([]byte("not-json")) })) defer ts.Close() @@ -71,7 +78,7 @@ func TestMediaAuthAttempt_BadJSON(t *testing.T) { } } -func TestMediaGetUserDetail_RetryOn5xx(t *testing.T) { +func TestMediaGetUserDetailRetryOn5xx(t *testing.T) { var calls int32 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := atomic.AddInt32(&calls, 1) @@ -79,7 +86,7 @@ func TestMediaGetUserDetail_RetryOn5xx(t *testing.T) { http.Error(w, "temp", http.StatusBadGateway) return } - w.Header().Set("Content-Type", "application/json") + w.Header().Set(headerContentType, contentTypeJSON) json.NewEncoder(w).Encode(map[string]any{ "Id": "u9", "Name": "neo", @@ -90,17 +97,17 @@ func TestMediaGetUserDetail_RetryOn5xx(t *testing.T) { out, err := mediaGetUserDetail("emby", ts.URL, "k", "u9") if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf(unexpectedErrFmt, err) } if out.ID != "u9" || out.Policy.IsDisabled { t.Fatalf("bad: %+v", out) } if atomic.LoadInt32(&calls) != 2 { - t.Fatalf("expected 2 calls, got %d", calls) + t.Fatalf(expectedCallsFmt, calls) } } -func TestMediaTokenStillValid_RetryOn5xx(t *testing.T) { +func TestMediaTokenStillValidRetryOn5xx(t *testing.T) { var calls int32 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := atomic.AddInt32(&calls, 1) @@ -114,12 +121,12 @@ func TestMediaTokenStillValid_RetryOn5xx(t *testing.T) { ok, err := mediaTokenStillValid("emby", ts.URL, "tok") if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf(unexpectedErrFmt, err) } if !ok { t.Fatal("expected ok=true") } if atomic.LoadInt32(&calls) != 2 { - t.Fatalf("expected 2 calls, got %d", calls) + t.Fatalf(expectedCallsFmt, calls) } }