API: Improve logging of bad request errors across all endpoints #271

lastzero · lastzero · commit 38da638f88a0 · 2025-07-10T09:38:36.000+02:00
Signed-off-by: Michael Mayer &lt;michael@photoprism.app&gt;
diff --git a/internal/api/abort.go b/internal/api/abort.go
@@ -103,10 +103,18 @@ func AbortUnexpectedError(c *gin.Context) {
 }
 
 func AbortBadRequest(c *gin.Context, errs ...error) {
+	// Log and attach validation errors to the context.
 	for _, err := range errs {
-		_ = c.Error(err)
+		if err != nil {
+			// Add error message to the debug logs.
+			log.Debugf("api-v1: %s", err)
+
+			// Attach error to the current context
+			_ = c.Error(err)
+		}
 	}
 
+	// Abort request with error 400.
 	Abort(c, http.StatusBadRequest, i18n.ErrBadRequest)
 }
 
diff --git a/internal/api/albums.go b/internal/api/albums.go
@@ -102,7 +102,7 @@ func CreateAlbum(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -194,8 +194,7 @@ func UpdateAlbum(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err = c.BindJSON(frm); err != nil {
-			log.Error(err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -439,7 +438,7 @@ func CloneAlbums(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err = c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -500,7 +499,7 @@ func AddPhotosToAlbum(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -531,8 +530,7 @@ func AddPhotosToAlbum(router *gin.RouterGroup) {
 		photos, err := query.SelectedPhotos(frm)
 
 		if err != nil {
-			log.Errorf("album: %s", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -611,7 +609,7 @@ func RemovePhotosFromAlbum(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/albums_search.go b/internal/api/albums_search.go
@@ -42,7 +42,7 @@ func SearchAlbums(router *gin.RouterGroup) {
 		// Abort if request params are invalid.
 		if err = c.MustBindWith(&frm, binding.Form); err != nil {
 			event.AuditWarn([]string{ClientIP(c), "session %s", "albums", "search", "form invalid", "%s"}, s.RefID, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/batch_albums.go b/internal/api/batch_albums.go
@@ -35,7 +35,7 @@ func BatchAlbumsDelete(router *gin.RouterGroup) {
 		var frm form.Selection
 
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/batch_labels.go b/internal/api/batch_labels.go
@@ -35,7 +35,7 @@ func BatchLabelsDelete(router *gin.RouterGroup) {
 		var frm form.Selection
 
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/batch_photos.go b/internal/api/batch_photos.go
@@ -45,7 +45,7 @@ func BatchPhotosArchive(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -116,7 +116,7 @@ func BatchPhotosRestore(router *gin.RouterGroup) {
 		var frm form.Selection
 
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -186,7 +186,7 @@ func BatchPhotosApprove(router *gin.RouterGroup) {
 		var frm form.Selection
 
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -246,7 +246,7 @@ func BatchPhotosPrivate(router *gin.RouterGroup) {
 		var frm form.Selection
 
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -313,7 +313,7 @@ func BatchPhotosDelete(router *gin.RouterGroup) {
 		var frm form.Selection
 
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/config_options.go b/internal/api/config_options.go
@@ -90,8 +90,7 @@ func SaveConfigOptions(router *gin.RouterGroup) {
 		}
 
 		if err := c.BindJSON(&v); err != nil {
-			log.Errorf("config: %s (bind json)", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/config_settings.go b/internal/api/config_settings.go
@@ -79,7 +79,7 @@ func SaveSettings(router *gin.RouterGroup) {
 
 			// Set values from request.
 			if err := c.BindJSON(settings); err != nil {
-				AbortBadRequest(c)
+				AbortBadRequest(c, err)
 				return
 			}
 
@@ -114,7 +114,7 @@ func SaveSettings(router *gin.RouterGroup) {
 
 			// Set values from request.
 			if err := c.BindJSON(settings); err != nil {
-				AbortBadRequest(c)
+				AbortBadRequest(c, err)
 				return
 			}
 
diff --git a/internal/api/faces.go b/internal/api/faces.go
@@ -70,7 +70,7 @@ func UpdateFace(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/faces_search.go b/internal/api/faces_search.go
@@ -40,7 +40,7 @@ func SearchFaces(router *gin.RouterGroup) {
 		err := c.MustBindWith(&frm, binding.Form)
 
 		if err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/feedback.go b/internal/api/feedback.go
@@ -36,7 +36,7 @@ func SendFeedback(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/folders_search.go b/internal/api/folders_search.go
@@ -58,7 +58,7 @@ func SearchFolders(router *gin.RouterGroup, urlPath, rootName, rootPath string)
 		err := c.MustBindWith(&frm, binding.Form)
 
 		if err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/import.go b/internal/api/import.go
@@ -68,7 +68,7 @@ func StartImport(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/index.go b/internal/api/index.go
@@ -52,7 +52,7 @@ func StartIndexing(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err := c.BindJSON(&frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/labels.go b/internal/api/labels.go
@@ -55,7 +55,7 @@ func UpdateLabel(router *gin.RouterGroup) {
 
 		// Set form values from request.
 		if frmErr = c.BindJSON(frm); frmErr != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, frmErr)
 			return
 		} else if frmErr = frm.Validate(); frmErr != nil {
 			AbortInvalidName(c)
diff --git a/internal/api/labels_search.go b/internal/api/labels_search.go
@@ -38,7 +38,7 @@ func SearchLabels(router *gin.RouterGroup) {
 		err := c.MustBindWith(&frm, binding.Form)
 
 		if err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/links.go b/internal/api/links.go
@@ -29,8 +29,7 @@ func UpdateLink(c *gin.Context) {
 
 	// Assign and validate request form values.
 	if err := c.BindJSON(&frm); err != nil {
-		log.Debugf("share: %s", err)
-		AbortBadRequest(c)
+		AbortBadRequest(c, err)
 		return
 	}
 
@@ -109,8 +108,7 @@ func CreateLink(c *gin.Context) {
 	var frm form.Link
 
 	if err := c.BindJSON(&frm); err != nil {
-		log.Debugf("share: %s", err)
-		AbortBadRequest(c)
+		AbortBadRequest(c, err)
 		return
 	}
 
diff --git a/internal/api/markers.go b/internal/api/markers.go
@@ -97,8 +97,7 @@ func CreateMarker(router *gin.RouterGroup) {
 
 		// Initialize form.
 		if err := c.BindJSON(&frm); err != nil {
-			log.Errorf("faces: %s (bind marker form)", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -113,8 +112,7 @@ func CreateMarker(router *gin.RouterGroup) {
 
 		// Validate form values.
 		if err = frm.Validate(); err != nil {
-			log.Errorf("faces: %s (validate new marker)", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		} else if frm.W <= 0 || frm.H <= 0 {
 			log.Errorf("faces: width and height must be greater than zero")
@@ -130,8 +128,7 @@ func CreateMarker(router *gin.RouterGroup) {
 
 		// Update marker from form values.
 		if err = marker.Create(); err != nil {
-			log.Errorf("faces: %s (create marker)", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -210,14 +207,14 @@ func UpdateMarker(router *gin.RouterGroup) {
 			return
 		} else if err = c.BindJSON(&frm); err != nil {
 			log.Errorf("faces: %s (bind marker form)", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
 		// Validate form values.
 		if err = frm.Validate(); err != nil {
 			log.Errorf("faces: %s (validate updated marker)", err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/oauth_revoke.go b/internal/api/oauth_revoke.go
@@ -75,7 +75,7 @@ func OAuthRevoke(router *gin.RouterGroup) {
 		// Get the auth token to be revoked from the submitted form values or the request header.
 		if err = c.ShouldBind(&frm); err != nil && authToken == "" {
 			event.AuditWarn([]string{clientIp, "oauth2", actor, action, "%s"}, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		} else if frm.Empty() {
 			frm.Token = authToken
diff --git a/internal/api/oauth_token.go b/internal/api/oauth_token.go
@@ -59,7 +59,7 @@ func OAuthToken(router *gin.RouterGroup) {
 			frm.ClientSecret = clientSecret
 		} else if err = c.ShouldBind(&frm); err != nil {
 			event.AuditWarn([]string{clientIp, "oauth2", actor, action, "%s"}, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/photo_label.go b/internal/api/photo_label.go
@@ -47,7 +47,7 @@ func AddPhotoLabel(router *gin.RouterGroup) {
 
 		// Assign and validate request form values.
 		if err = c.BindJSON(frm); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		} else if err = frm.Validate(); err != nil {
 			AbortInvalidName(c)
@@ -221,7 +221,7 @@ func UpdatePhotoLabel(router *gin.RouterGroup) {
 		}
 
 		if err = c.BindJSON(label); err != nil {
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/photos_search.go b/internal/api/photos_search.go
@@ -49,7 +49,7 @@ func SearchPhotos(router *gin.RouterGroup) {
 		// Abort if request params are invalid.
 		if err = c.MustBindWith(&frm, binding.Form); err != nil {
 			event.AuditWarn([]string{ClientIP(c), "session %s", string(acl.ResourcePhotos), "form invalid", "%s"}, s.RefID, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return frm, s, err
 		}
 
@@ -85,7 +85,7 @@ func SearchPhotos(router *gin.RouterGroup) {
 		// Ok?
 		if err != nil {
 			event.AuditWarn([]string{ClientIP(c), "session %s", string(acl.ResourcePhotos), "search", "%s"}, s.RefID, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -114,7 +114,7 @@ func SearchPhotos(router *gin.RouterGroup) {
 
 		if err != nil {
 			event.AuditWarn([]string{ClientIP(c), "session %s", string(acl.ResourcePhotos), "view", "%s"}, s.RefID, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/photos_search_geo.go b/internal/api/photos_search_geo.go
@@ -49,7 +49,7 @@ func SearchGeo(router *gin.RouterGroup) {
 		// Abort if request params are invalid.
 		if err = c.MustBindWith(&frm, binding.Form); err != nil {
 			event.AuditWarn([]string{ClientIP(c), "session %s", string(acl.ResourcePlaces), "form invalid", "%s"}, s.RefID, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
@@ -74,7 +74,7 @@ func SearchGeo(router *gin.RouterGroup) {
 		// Ok?
 		if err != nil {
 			event.AuditWarn([]string{ClientIP(c), "session %s", string(acl.ResourcePlaces), "search", "%s"}, s.RefID, err)
-			AbortBadRequest(c)
+			AbortBadRequest(c, err)
 			return
 		}
 
diff --git a/internal/api/services.go b/internal/api/services.go
diff --git a/internal/api/services_search.go b/internal/api/services_search.go
diff --git a/internal/api/services_upload.go b/internal/api/services_upload.go
diff --git a/internal/api/session_create.go b/internal/api/session_create.go
diff --git a/internal/api/subjects.go b/internal/api/subjects.go
diff --git a/internal/api/subjects_search.go b/internal/api/subjects_search.go
diff --git a/internal/api/swagger.json b/internal/api/swagger.json
diff --git a/internal/api/users_sessions.go b/internal/api/users_sessions.go
diff --git a/internal/api/users_update.go b/internal/api/users_update.go
diff --git a/internal/api/users_upload.go b/internal/api/users_upload.go
diff --git a/internal/api/zip.go b/internal/api/zip.go

Original file line number	Diff line number	Diff line change
`@@ -103,10 +103,18 @@ func AbortUnexpectedError(c *gin.Context) {`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`func AbortBadRequest(c *gin.Context, errs ...error) {`
	`106`	`+ // Log and attach validation errors to the context.`
`106`	`107`	`for _, err := range errs {`
`107`		`- _ = c.Error(err)`
	`108`	`+ if err != nil {`
	`109`	`+ // Add error message to the debug logs.`
	`110`	`+ log.Debugf("api-v1: %s", err)`
	`111`	`+`
	`112`	`+ // Attach error to the current context`
	`113`	`+ _ = c.Error(err)`
	`114`	`+ }`
`108`	`115`	`}`
`109`	`116`
	`117`	`+ // Abort request with error 400.`
`110`	`118`	`Abort(c, http.StatusBadRequest, i18n.ErrBadRequest)`
`111`	`119`	`}`
`112`	`120`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ func SearchAlbums(router *gin.RouterGroup) {`
`42`	`42`	`// Abort if request params are invalid.`
`43`	`43`	`if err = c.MustBindWith(&frm, binding.Form); err != nil {`
`44`	`44`	`event.AuditWarn([]string{ClientIP(c), "session %s", "albums", "search", "form invalid", "%s"}, s.RefID, err)`
`45`		`- AbortBadRequest(c)`
	`45`	`+ AbortBadRequest(c, err)`
`46`	`46`	`return`
`47`	`47`	`}`
`48`	`48`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ func BatchAlbumsDelete(router *gin.RouterGroup) {`
`35`	`35`	`var frm form.Selection`
`36`	`36`
`37`	`37`	`if err := c.BindJSON(&frm); err != nil {`
`38`		`- AbortBadRequest(c)`
	`38`	`+ AbortBadRequest(c, err)`
`39`	`39`	`return`
`40`	`40`	`}`
`41`	`41`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ func BatchLabelsDelete(router *gin.RouterGroup) {`
`35`	`35`	`var frm form.Selection`
`36`	`36`
`37`	`37`	`if err := c.BindJSON(&frm); err != nil {`
`38`		`- AbortBadRequest(c)`
	`38`	`+ AbortBadRequest(c, err)`
`39`	`39`	`return`
`40`	`40`	`}`
`41`	`41`
Original file line number	Diff line number	Diff line change
`@@ -90,8 +90,7 @@ func SaveConfigOptions(router *gin.RouterGroup) {`
`90`	`90`	`}`
`91`	`91`
`92`	`92`	`if err := c.BindJSON(&v); err != nil {`
`93`		`- log.Errorf("config: %s (bind json)", err)`
`94`		`- AbortBadRequest(c)`
	`93`	`+ AbortBadRequest(c, err)`
`95`	`94`	`return`
`96`	`95`	`}`
`97`	`96`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ func SaveSettings(router *gin.RouterGroup) {`
`79`	`79`
`80`	`80`	`// Set values from request.`
`81`	`81`	`if err := c.BindJSON(settings); err != nil {`
`82`		`- AbortBadRequest(c)`
	`82`	`+ AbortBadRequest(c, err)`
`83`	`83`	`return`
`84`	`84`	`}`
`85`	`85`
`@@ -114,7 +114,7 @@ func SaveSettings(router *gin.RouterGroup) {`
`114`	`114`
`115`	`115`	`// Set values from request.`
`116`	`116`	`if err := c.BindJSON(settings); err != nil {`
`117`		`- AbortBadRequest(c)`
	`117`	`+ AbortBadRequest(c, err)`
`118`	`118`	`return`
`119`	`119`	`}`
`120`	`120`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ func UpdateFace(router *gin.RouterGroup) {`
`70`	`70`
`71`	`71`	`// Assign and validate request form values.`
`72`	`72`	`if err := c.BindJSON(&frm); err != nil {`
`73`		`- AbortBadRequest(c)`
	`73`	`+ AbortBadRequest(c, err)`
`74`	`74`	`return`
`75`	`75`	`}`
`76`	`76`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ func SearchFaces(router *gin.RouterGroup) {`
`40`	`40`	`err := c.MustBindWith(&frm, binding.Form)`
`41`	`41`
`42`	`42`	`if err != nil {`
`43`		`- AbortBadRequest(c)`
	`43`	`+ AbortBadRequest(c, err)`
`44`	`44`	`return`
`45`	`45`	`}`
`46`	`46`