return more helpful logging on server error

core/requests.go

@@ -240,7 +240,7 @@ func makeRequest(ctx context.Context, reqFunc func(context.Context, string) (*re
 	// Generate the HTTP request using the provided request function.
 	req, err := reqFunc(ctx, token.Value)
 	if err != nil {
-		return nil, err
+		return nil, i18n.Errorf("failed to create API request with token: %w", url, err)
 	}
 
 	start := time.Now()
@@ -248,25 +248,25 @@ func makeRequest(ctx context.Context, reqFunc func(context.Context, string) (*re
 	end := time.Now()
 
 	if err != nil {
-		return nil, i18n.Errorf("failed to make request: %w", err)
+		return nil, i18n.Errorf("failed to make HTTP request: %w", url, err)
 	}
 	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, err
+		return nil, i18n.Errorf("failed to read response body: %w", url, err)
 	}
 
 	audit.LogAPIRoundTrip(audit.APIRequestSpan{
-		StartTime: start,
-		EndTime:   end,
-		RequestId: request_context.GetFromContext(ctx).RequestId,
-		Response:  resp,
-		Error:     err,
-		Method:    req.Method,
-		Url:       url,
-		Token:     token.Value,
-		Body:      string(body),
+		StartTime:  start,
+		EndTime:    end,
+		RequestId:  request_context.GetFromContext(ctx).RequestId,
+		Response:   resp,
+		ErrorField: err,
+		Method:     req.Method,
+		Url:        url,
+		Token:      token.Value,
+		Body:       string(body),
 	})
 
 	return &SimpleHTTPResponse{

server/audit/audit_log.go

@@ -20,19 +20,28 @@ var RecordedRequestSpans = []Span{}
 func LogAndRecord(requestSpan Span) {
 	duration := float64(Duration(requestSpan)) / float64(time.Second)
 
-	Logger.Info("Request",
-		// Zap already prefixes the log with a timestamp
-		// zap.String("timestamp", span.GetStartTime().Format(time.RFC3339)),
-		zap.String("component", requestSpan.Component()),
-		zap.Any("action", requestSpan.Action()),
-		zap.Any("outcome", requestSpan.Outcome()),
-		zap.Float64("duration", duration),
-	)
+	// Determine the log level based on the presence of an error
+	// var logger *zap.Logger
+	if requestSpan.Error() != nil {
+		Logger.Error("Request",
+			zap.String("component", requestSpan.Component()),
+			zap.Any("action", requestSpan.Action()),
+			zap.Any("outcome", requestSpan.Outcome()),
+			zap.Float64("duration", duration),
+		)
+	} else {
+		Logger.Info("Request",
+			zap.String("component", requestSpan.Component()),
+			zap.Any("action", requestSpan.Action()),
+			zap.Any("outcome", requestSpan.Outcome()),
+			zap.Float64("duration", duration),
+		)
+	}
 
 	// If MaxRecordedCount is set, manage the RecordedRequestSpans slice
 	if MaxRecordedCount != 0 {
 		// Remove the oldest span if we're at capacity
-		if len(RecordedRequestSpans)+1 == MaxRecordedCount {
+		if len(RecordedRequestSpans)+1 > MaxRecordedCount {
 			RecordedRequestSpans = RecordedRequestSpans[1:]
 		}
 		// Append the new span
@@ -41,37 +50,19 @@ func LogAndRecord(requestSpan Span) {
 }
 
 // LogServerRoundTrip logs and records a ServerRequestSpan.
-// It also logs any internal server errors that occurred during the request.
 func LogServerRoundTrip(requestSpan ServerRequestSpan) {
-	if requestSpan.Error != nil {
-		Logger.Error("Internal Server Error",
-			zap.Error(requestSpan.Error),
-			zap.String("requestId", requestSpan.RequestId),
-		)
-	}
-
 	LogAndRecord(requestSpan)
 }
 
 // LogAPIRoundTrip logs and records an APIRequestSpan.
-// It handles saving the response body to a file if enabled and logs warnings for non-2xx status codes.
 func LogAPIRoundTrip(requestSpan APIRequestSpan) {
-	if requestSpan.Response != nil {
-		// Save response body to file if enabled and body is not empty
-		if requestSpan.Body != "" && optionSaveResponse {
-			var err error
-			requestSpan.ResponseFilename, err = writeResponseBodyToFile(requestSpan.Body)
-			if err != nil {
-				Logger.Error("Failed to save response to file",
-					zap.Error(err),
-					zap.String("requestId", requestSpan.RequestId),
-				)
-			}
-		}
-		// Log a warning for non-2xx status codes
-		if !(300 > requestSpan.Response.StatusCode && requestSpan.Response.StatusCode >= 200) {
-			Logger.Warn("Non-2xx response from pixiv",
-				zap.Int("status", requestSpan.Response.StatusCode),
+	// Save response body to file if enabled and conditions are met
+	if requestSpan.Response != nil && requestSpan.Body != "" && optionSaveResponse {
+		var err error
+		requestSpan.ResponseFilename, err = writeResponseBodyToFile(requestSpan.Body)
+		if err != nil {
+			Logger.Error("Failed to save response to file",
+				zap.Error(err),
 				zap.String("requestId", requestSpan.RequestId),
 			)
 		}

server/audit/spans.go

@@ -7,6 +7,7 @@ import (
 	"codeberg.org/vnpower/pixivfe/v2/i18n"
 )
 
+// Span interface now includes an Error method
 type Span interface {
 	GetStartTime() time.Time
 	GetEndTime() time.Time
@@ -14,6 +15,7 @@ type Span interface {
 	Component() string
 	Action() map[string]interface{}
 	Outcome() map[string]interface{}
+	Error() error
 }
 
 func Duration(span Span) time.Duration {
@@ -29,7 +31,7 @@ type ServerRequestSpan struct {
 	Status     int
 	Referer    string
 	RemoteAddr string
-	Error      error
+	ErrorField error
 }
 
 func (span ServerRequestSpan) GetStartTime() time.Time {
@@ -57,22 +59,28 @@ func (span ServerRequestSpan) Action() map[string]interface{} {
 
 func (span ServerRequestSpan) Outcome() map[string]interface{} {
 	outcome := map[string]interface{}{
-		"status": span.Status,
+		"status": "success",
 		"error":  "<none>",
 		"locale": i18n.GetLocale(),
 	}
-	if span.Error != nil {
-		outcome["error"] = span.Error.Error()
+	if span.ErrorField != nil {
+		outcome["status"] = "error"
+		outcome["error"] = span.ErrorField.Error()
 	}
 	return outcome
 }
 
+// Implement the Error method for ServerRequestSpan
+func (span ServerRequestSpan) Error() error {
+	return span.ErrorField
+}
+
 type APIRequestSpan struct {
 	StartTime        time.Time
 	EndTime          time.Time
 	RequestId        string
 	Response         *http.Response `json:"-"`
-	Error            error
+	ErrorField       error
 	Method           string
 	Url              string
 	Token            string
@@ -106,16 +114,27 @@ func (span APIRequestSpan) Action() map[string]interface{} {
 
 func (span APIRequestSpan) Outcome() map[string]interface{} {
 	outcome := map[string]interface{}{
-		"status": "success",
+		"status": "<unknown>", // Default status
 		"error":  "<none>",
 		"locale": i18n.GetLocale(),
 	}
-	if span.Error != nil {
+
+	if span.ErrorField != nil {
 		outcome["status"] = "error"
-		outcome["error"] = span.Error.Error()
-	}
-	if span.Response != nil {
+		outcome["error"] = span.ErrorField.Error()
+	} else if span.Response != nil {
+		if span.Response.StatusCode >= 200 && span.Response.StatusCode < 300 {
+			outcome["status"] = "success"
+		} else {
+			outcome["status"] = "warning"
+		}
 		outcome["status_code"] = span.Response.StatusCode
 	}
+
 	return outcome
 }
+
+// Implement the Error method for APIRequestSpan
+func (span APIRequestSpan) Error() error {
+	return span.ErrorField
+}

server/middleware/logger.go

@@ -67,7 +67,7 @@ func LogRequest(h http.Handler) http.Handler {
 				Status:     w.statusCode,
 				Referer:    r.Referer(),
 				RemoteAddr: r.RemoteAddr,
-				Error:      request_context.Get(r).CaughtError,
+				ErrorField: request_context.Get(r).CaughtError,
 			})
 		}
 	})