Explorar el Código

fix(karakeep): report the HTTP status when saving an entry fails

SaveURL rejected the response on its Content-Type before it looked at
the status code, so any error the server returned as HTML (a reverse
proxy 502, an auth redirect, a wrong endpoint) surfaced as
"unexpected content type response: text/html; charset=utf-8" with no
mention of the status. Users had nothing to act on.

The check was also the only one of its kind in internal/integration:
every other client, including attachTags in this same file, decides on
the status code alone and lets json.Unmarshal report a body it cannot
read. Drop it so a failed save names its status.
Aditya Raj Singh hace 1 mes
padre
commit
249b340ce2

+ 0 - 4
internal/integration/karakeep/karakeep.go

@@ -136,10 +136,6 @@ func (c *Client) SaveURL(entryURL string) error {
 		return fmt.Errorf("karakeep: failed to parse response: %s", err)
 	}
 
-	if resp.Header.Get("Content-Type") != "application/json" {
-		return fmt.Errorf("karakeep: unexpected content type response: %s", resp.Header.Get("Content-Type"))
-	}
-
 	if resp.StatusCode != http.StatusCreated {
 		var errResponse errorResponse
 		if err := json.Unmarshal(responseBody, &errResponse); err != nil {

+ 90 - 0
internal/integration/karakeep/karakeep_test.go

@@ -0,0 +1,90 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package karakeep
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"miniflux.app/v2/internal/config"
+)
+
+func TestSaveURL(t *testing.T) {
+	configureIntegrationAllowPrivateNetworksOption(t)
+
+	tests := []struct {
+		name           string
+		serverResponse func(w http.ResponseWriter, r *http.Request)
+		errContains    string
+	}{
+		{
+			name: "successful save",
+			serverResponse: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusCreated)
+				w.Write([]byte(`{"id":"bookmark-id"}`))
+			},
+		},
+		{
+			name: "json error response reports the status and error code",
+			serverResponse: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(http.StatusUnauthorized)
+				w.Write([]byte(`{"code":"UNAUTHORIZED","error":"invalid token"}`))
+			},
+			errContains: "status=401",
+		},
+		{
+			name: "html error response reports the status",
+			serverResponse: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "text/html; charset=utf-8")
+				w.WriteHeader(http.StatusNotFound)
+				w.Write([]byte("<!DOCTYPE html><title>Not Found</title>"))
+			},
+			errContains: "status=404",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			server := httptest.NewServer(http.HandlerFunc(tt.serverResponse))
+			defer server.Close()
+
+			err := NewClient("test-api-key", server.URL, "").SaveURL("https://example.com/article")
+
+			if tt.errContains == "" {
+				if err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
+				return
+			}
+			if err == nil {
+				t.Fatalf("expected error, got none")
+			}
+			if !strings.Contains(err.Error(), tt.errContains) {
+				t.Fatalf("expected error containing %q, got %q", tt.errContains, err.Error())
+			}
+		})
+	}
+}
+
+func configureIntegrationAllowPrivateNetworksOption(t *testing.T) {
+	t.Helper()
+
+	t.Setenv("INTEGRATION_ALLOW_PRIVATE_NETWORKS", "1")
+
+	configParser := config.NewConfigParser()
+	parsedOptions, err := configParser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf("Unable to configure test options: %v", err)
+	}
+
+	previousOptions := config.Opts
+	config.Opts = parsedOptions
+	t.Cleanup(func() {
+		config.Opts = previousOptions
+	})
+}