Skip to content
10 changes: 10 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,16 @@ func (c *ScreenshotHandlingConfig) ResolvedMaxMenuItems() int {
return c.MaxMenuItems
}

// ResolvedMinSuccessfulUploads returns how many images a host must publish for
// a tracker before a partially failed image-host batch still counts as usable.
// Zero or negative disables the allowance, so any failed image fails the batch.
func (c *ScreenshotHandlingConfig) ResolvedMinSuccessfulUploads() int {
if c.MinSuccessfulUploads <= 0 {
return 0
}
return c.MinSuccessfulUploads
}

type DescriptionSettingsConfig struct {
AddLogo bool `yaml:"add_logo"`
LogoSize int `yaml:"logo_size"`
Expand Down
2 changes: 2 additions & 0 deletions internal/config/defaults/example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ screenshot_handling:
screens: 4
# Automatic DVD menu capture is opt-in. This caps stored distinct menu screens (1-32).
max_menu_items: 6
# When an image host drops some uploads, the batch still counts as usable once
# this many images are published. Zero requires every image to succeed.
min_successful_image_uploads: 3
cutoff_screens: 1
frame_overlay: false
Expand Down
36 changes: 36 additions & 0 deletions internal/core/media.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,10 @@ func (m *mediaModule) uploadImagesToTarget(
len(images),
)
uploaded, err := m.images.Upload(progressCtx, imageHostingSubject(meta), target.Host, target.UsageScope, images)
if err != nil && m.partialHostUploadIsUsable(target, len(images), len(uploaded), err) {
emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), nil)
return uploaded, nil
}
emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err)
return wrapCoreResult(uploaded, err)
}
Expand Down Expand Up @@ -703,13 +707,45 @@ func (m *mediaModule) uploadImagesToTarget(
)
uploaded, err := m.images.Upload(progressCtx, imageHostingSubject(meta), target.Host, target.UsageScope, missing)
results = append(results, uploaded...)
if err != nil && m.partialHostUploadIsUsable(target, len(images), len(results), err) {
emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), nil)
return results, nil
}
emitCoreImageUploadResult(progressCtx, progressTarget, len(uploaded), err)
if err != nil {
return results, fmt.Errorf("core: %w", err)
}
return results, nil
}

// partialHostUploadIsUsable reports whether a partially failed host batch still
// published enough images for the target's trackers to upload with. The
// allowance is the configured min_successful_image_uploads floor, which is
// disabled at zero so every failed image keeps failing the batch. Requested
// counts below the floor cannot clear it, so those batches stay strict.
func (m *mediaModule) partialHostUploadIsUsable(
target trackers.ImageUploadTarget,
requested int,
published int,
err error,
) bool {
minimum := m.cfg.ScreenshotHandling.ResolvedMinSuccessfulUploads()
if minimum <= 0 || published < minimum || requested < minimum {
return false
}
m.logger.Warnf(
"core: accepting partial image host upload host=%s tracker=%s trackers=%v requested=%d published=%d minimum=%d decision=continue err=%s",
target.Host,
m.imageHostOwnerLogValue(target.Host),
target.Trackers,
requested,
published,
minimum,
logging.SanitizeMessage(uploadFailureMessage(err)),
)
return true
}

func emitCoreImageUploadResult(
ctx context.Context,
target api.ImageUploadProgressTarget,
Expand Down
132 changes: 132 additions & 0 deletions internal/core/media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,3 +362,135 @@ func sortedCallHosts(calls []imageHostCall) []string {
slices.Sort(hosts)
return hosts
}

// partialImageHostingService publishes a fixed number of images and then fails
// the batch, mirroring a host that drops individual uploads under load.
type partialImageHostingService struct {
published int
}

func (*partialImageHostingService) ListCandidates(context.Context, api.ImageHostingSubject) ([]api.ScreenshotImage, error) {
return nil, nil
}

func (s *partialImageHostingService) Upload(
_ context.Context,
_ api.ImageHostingSubject,
host string,
usageScope string,
images []api.ScreenshotImage,
) ([]api.UploadedImageLink, error) {
published := min(s.published, len(images))
links := make([]api.UploadedImageLink, 0, published)
for _, image := range images[:published] {
links = append(links, api.UploadedImageLink{
ImagePath: image.Path,
Host: host,
UsageScope: usageScope,
RawURL: "https://images.example.invalid/" + host,
})
}
if published == len(images) {
return links, nil
}
return links, fmt.Errorf("image hosting: %d of %d uploads failed", len(images)-published, len(images))
}

func TestUploadImagesAcceptsPartialHostBatchAtConfiguredMinimum(t *testing.T) {
t.Parallel()

images := make([]api.ScreenshotImage, 0, 6)
for index := range 6 {
images = append(images, api.ScreenshotImage{Path: fmt.Sprintf("screen%d.png", index)})
}
target := trackers.ImageUploadTarget{
Host: "pixhost",
UsageScope: "global",
Trackers: []string{"ONE"},
}

for _, testCase := range []struct {
name string
minimum int
published int
wantLinks int
wantFailure bool
}{
{
name: "above minimum",
minimum: 3,
published: 5,
wantLinks: 5,
},
{
name: "at minimum",
minimum: 3,
published: 3,
wantLinks: 3,
},
{
name: "below minimum",
minimum: 3,
published: 2,
wantFailure: true,
},
{
name: "allowance disabled",
minimum: 0,
published: 5,
wantFailure: true,
},
{
name: "minimum above requested",
minimum: 8,
published: 5,
wantFailure: true,
},
} {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

module := &mediaModule{
cfg: config.Config{
ImageHosting: config.ImageHostingConfig{Host1: "pixhost"},
ScreenshotHandling: config.ScreenshotHandlingConfig{MinSuccessfulUploads: testCase.minimum},
},
images: &partialImageHostingService{published: testCase.published},
logger: &recordingMediaLogger{},
registry: mediaImageHostRegistry(t),
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
result, err := module.uploadImagesToTargetsWithFallback(
context.Background(),
api.UploadSubject{SourcePath: "Example.Release.2026.mkv"},
"pixhost",
nil,
[]trackers.ImageUploadTarget{target},
images,
)
if err != nil {
t.Fatalf("upload images: %v", err)
}
if testCase.wantFailure {
if len(result.Failures) != 1 || !slices.Equal(result.Failures[0].Trackers, []string{"ONE"}) {
t.Fatalf("expected a tracker-scoped failure, got %#v", result.Failures)
}
if !slices.Contains(result.FailedHosts, "pixhost") {
t.Fatalf("expected pixhost to be marked failed, got %v", result.FailedHosts)
}
return
}
if len(result.Failures) != 0 {
t.Fatalf("partial batch above the minimum must not report a failure: %#v", result.Failures)
}
if len(result.FailedHosts) != 0 {
t.Fatalf("partial batch above the minimum must not mark the host failed: %v", result.FailedHosts)
}
if len(result.Links) != testCase.wantLinks {
t.Fatalf("published links = %d, want %d", len(result.Links), testCase.wantLinks)
}
if len(result.Attempts) != 1 || result.Attempts[0].Failure != nil {
t.Fatalf("attempt results = %#v", result.Attempts)
}
})
}
}
13 changes: 13 additions & 0 deletions internal/releaseworkflow/eligibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,24 @@ func resolveDownstreamTrackerSet(
if !ok || media.Revision != state.Workflow.Media.Revision {
return downstreamTrackerSet{}, fmt.Errorf("%w: retained media is stale", ErrInvalidTransition)
}
blocked := make([]string, 0, len(base))
for trackerID := range base {
if _, failed := TrackerImageHostFailure(media, trackerID); failed {
delete(base, trackerID)
blocked = append(blocked, string(trackerID))
}
}
// An empty set here would otherwise travel silently into the upload
// plan and only surface as a contract validation error about missing
// target tracker IDs, which names neither the stage nor the cause.
if len(base) == 0 && len(blocked) > 0 {
slices.Sort(blocked)
return downstreamTrackerSet{}, fmt.Errorf(
"%w: image hosting failed for every downstream tracker (%s)",
ErrInvalidTransition,
strings.Join(blocked, ", "),
)
}
}
selected := base
if len(requested) > 0 {
Expand Down
17 changes: 17 additions & 0 deletions internal/releaseworkflow/eligibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,23 @@ TrackerID: "BETA",
if err != nil || !slices.Equal(webTargets.TrackerIDs(), []api.TrackerID{"ALPHA"}) {
t.Fatalf("WebUI upload targets after image-host failure = %#v err=%v", webTargets, err)
}

// Losing every tracker to image hosting must name the cause here instead of
// handing an empty target set to upload-plan contract validation.
allFailed := media
allFailed.Failures = append(append([]api.WorkflowFailure(nil), media.Failures...), api.WorkflowFailure{
Failure: api.OperationFailure{Operation: api.OperationKindImageHosting},
TrackerID: "ALPHA",
})
state.Media[media.ID] = allFailed
emptied, err := resolveDownstreamTrackerSet(&state, nil, downstreamStageUpload, now)
if !errors.Is(err, ErrInvalidTransition) || len(emptied.TrackerIDs()) != 0 {
t.Fatalf("upload targets with every tracker image-host blocked = %#v err=%v", emptied, err)
}
if !strings.Contains(err.Error(), "ALPHA, BETA") {
t.Fatalf("image-host exhaustion error must name the blocked trackers: %v", err)
}
state.Media[media.ID] = media
state.Workflow.Media = nil

state.TrackerDecisionMode = TrackerDecisionModePostDupeGate
Expand Down
Loading