Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adding --skip-problem-assets global param #804

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding --skip-problem-assets global param #804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 55 additions & 15 deletions app/cmd/upload/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand All @@ -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
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions app/cmd/upload/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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))
Expand Down
20 changes: 18 additions & 2 deletions immich/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down