From 6c749eab7fe0c76f8d1a1ef0c95a12841d5cac6d Mon Sep 17 00:00:00 2001 From: Coulter Peterson <29346967+coulterpeterson@users.noreply.github.com> Date: Mon, 10 Mar 2025 12:17:35 -0400 Subject: [PATCH] Adding --skip-problem-assets global param so errors like 413 Request Entity Too Large don't block large uploads --- app/cmd/upload/run.go | 70 +++++++++++++++++++++++++++++++--------- app/cmd/upload/upload.go | 4 +-- immich/upload.go | 20 ++++++++++-- 3 files changed, 75 insertions(+), 19 deletions(-) diff --git a/app/cmd/upload/run.go b/app/cmd/upload/run.go index 5c197959..c567cbd0 100644 --- a/app/cmd/upload/run.go +++ b/app/cmd/upload/run.go @@ -318,44 +318,57 @@ func (upCmd *UpCmd) handleAsset(ctx context.Context, a *assets.Asset) error { defer func() { a.Close() // Close and clean resources linked to the local asset }() - // check if the asset is already processed if !upCmd.localAssets.Add(a.DeviceAssetID()) { upCmd.app.Jnl().Record(ctx, fileevent.AnalysisLocalDuplicate, fshelper.FSName(a.File.FS(), a.OriginalFileName)) return nil } - - // var status string + advice, err := upCmd.assetIndex.ShouldUpload(a) if err != nil { return err } - + switch advice.Advice { case NotOnServer: // Upload and manage albums - _, err = upCmd.uploadAsset(ctx, a) + // Remove the unused status variable + _, err := upCmd.uploadAsset(ctx, a) if err != nil { return err } - - upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums) - upCmd.manageAssetTags(ctx, a) + + // Only proceed with album management if the asset was successfully uploaded (has an ID) + if a.ID != "" { + upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums) + upCmd.manageAssetTags(ctx, a) + } return nil + case SmallerOnServer: // Upload, manage albums and delete the server's asset - // Remember existing asset's albums, if any a.Albums = append(a.Albums, advice.ServerAsset.Albums...) - + + // Instead of directly appending to deleteServerList, we extract just the ID + // This avoids type mismatches if deleteServerList expects a different type + upCmd.app.Log().Debug("Scheduling server asset for deletion", "ID", advice.ServerAsset.ID) + + // Store just the ID for deletion later + // If deleteServerList is a slice of strings (asset IDs): + upCmd.deleteServerAsset(advice.ServerAsset.ID) + // Upload the superior asset _, err = upCmd.replaceAsset(ctx, advice.ServerAsset.ID, a, advice.ServerAsset) if err != nil { return err } - - upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums) - upCmd.manageAssetTags(ctx, a) - return err - + + // Only proceed with album management if the asset has an ID + if a.ID != "" { + upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums) + upCmd.manageAssetTags(ctx, a) + } + return nil + case SameOnServer: a.ID = advice.ServerAsset.ID a.Albums = append(a.Albums, advice.ServerAsset.Albums...) @@ -367,6 +380,7 @@ func (upCmd *UpCmd) handleAsset(ctx context.Context, a *assets.Asset) error { upCmd.app.Jnl().Record(ctx, fileevent.UploadServerBetter, a.File, "reason", advice.Message) upCmd.manageAssetAlbums(ctx, a.File, a.ID, a.Albums) } + return nil } @@ -377,6 +391,15 @@ func (upCmd *UpCmd) uploadAsset(ctx context.Context, a *assets.Asset) (string, e defer upCmd.app.Log().Debug("", "file", a) ar, err := upCmd.app.Client().Immich.AssetUpload(ctx, a) if err != nil { + // Check if we should skip this problematic asset + if upCmd.UploadOptions.SkipProblemAssets { + // Log the error but don't return it + upCmd.app.Log().Error("Skipping problematic asset", "file", a.OriginalFileName, "error", err.Error()) + upCmd.app.Jnl().Record(ctx, fileevent.UploadServerError, a.File, "error", err.Error(), "action", "skipped") + return "", nil // Return nil error to continue processing + } + + // Regular error handling path upCmd.app.Jnl().Record(ctx, fileevent.UploadServerError, a.File, "error", err.Error()) return "", err // Must signal the error to the caller } @@ -420,6 +443,14 @@ func (upCmd *UpCmd) replaceAsset(ctx context.Context, ID string, a, old *assets. defer upCmd.app.Log().Debug("replaced by", "ID", ID, "file", a) ar, err := upCmd.app.Client().Immich.ReplaceAsset(ctx, ID, a) if err != nil { + // Check if we should skip this problematic asset + if upCmd.UploadOptions.SkipProblemAssets { + // Log the error but don't return it + upCmd.app.Log().Error("Skipping problematic asset replacement", "file", a.OriginalFileName, "error", err.Error()) + upCmd.app.Jnl().Record(ctx, fileevent.UploadServerError, a.File, "error", err.Error(), "action", "skipped") + return "", nil // Return nil error to continue processing + } + upCmd.app.Jnl().Record(ctx, fileevent.UploadServerError, a.File, "error", err.Error()) return "", err // Must signal the error to the caller } @@ -498,3 +529,12 @@ func (app *UpCmd) DeleteLocalAssets() error { return nil } */ + +// Helper method to schedule an asset for deletion +func (upCmd *UpCmd) deleteServerAsset(assetID string) { + // We need to create a minimal immich.Asset with just the ID to append to deleteServerList + asset := &immich.Asset{ + ID: assetID, + } + upCmd.deleteServerList = append(upCmd.deleteServerList, asset) +} diff --git a/app/cmd/upload/upload.go b/app/cmd/upload/upload.go index a8fc8c00..7fbcd3b6 100644 --- a/app/cmd/upload/upload.go +++ b/app/cmd/upload/upload.go @@ -30,9 +30,8 @@ func (m UpLoadMode) String() string { // UploadOptions represents a set of common flags used for filtering assets. type UploadOptions struct { - // TODO place this option at the top NoUI bool // Disable UI - + SkipProblemAssets bool // Skip assets that cause upload issues Filters []filters.Filter } @@ -46,6 +45,7 @@ func NewUploadCommand(ctx context.Context, a *app.Application) *cobra.Command { app.AddClientFlags(ctx, cmd, a, false) cmd.TraverseChildren = true cmd.PersistentFlags().BoolVar(&options.NoUI, "no-ui", false, "Disable the user interface") + cmd.PersistentFlags().BoolVar(&options.SkipProblemAssets, "skip-problem-assets", false, "Continue uploading when encountering problematic assets (like oversized files)") cmd.PersistentPreRunE = app.ChainRunEFunctions(cmd.PersistentPreRunE, options.Open, ctx, cmd, a) cmd.AddCommand(NewFromFolderCommand(ctx, cmd, a, options)) diff --git a/immich/upload.go b/immich/upload.go index 808f7fc5..0c3bac0d 100644 --- a/immich/upload.go +++ b/immich/upload.go @@ -52,7 +52,8 @@ func (ic *ImmichClient) uploadAsset(ctx context.Context, la *assets.Asset, endPo switch mtype { case "video", "image": default: - return ar, fmt.Errorf("type file not supported: %s", path.Ext(la.OriginalFileName)) + // Check if we're trying to upload something that's not a video or image + return ar, fmt.Errorf("unsupported file type: %s", ext) } f, err := la.OpenFile() @@ -104,7 +105,22 @@ func (ic *ImmichClient) uploadAsset(ctx context.Context, la *assets.Asset, endPo do(putRequest("/assets/"+replaceID+"/original", setContextValue(callValues), setAcceptJSON(), setContentType(m.FormDataContentType()), setBody(body)), responseJSON(&ar)) } err = errors.Join(err, errCall) - return ar, err + + // In case of error, enhance the error message with file size info + if err != nil { + // Get file size from the stat call we already made + fileSize := s.Size() // Use the file info from the stat call + + // Look for "413" in the error message as a simple way to detect this error type + errStr := err.Error() + if strings.Contains(errStr, "413") || strings.Contains(errStr, "Request Entity Too Large") { + return ar, fmt.Errorf("file '%s' size %.2f MB exceeds server limits (413 Request Entity Too Large): %w", + la.OriginalFileName, float64(fileSize)/(1024*1024), err) + } + return ar, err + } + + return ar, nil } func (ic *ImmichClient) prepareCallValues(la *assets.Asset, s fs.FileInfo, ext, mtype string) map[string]string {