fix(scraper): prevent nil pointer dereference in scrapeScene

[dutchdevil-83]

Description

scrapeScene declares ret as a nil *models.ScrapedScene and only initializes it when results are found. It then unconditionally calls processSceneRelationships, which dereferences ret at line 89:

ret.Performers = s.processPerformers(ctx, scenePerformersMap, q)

This causes a panic when a scraper returns no direct scene fields but the function continues to process relationships.

Root cause

// before
var ret *models.ScrapedScene        // nil
if len(results) > 0 {
    ret = results[0].scrapedScene() // only set when results exist
}
hasRelationships := s.processSceneRelationships(ctx, q, 0, ret) // panics if ret == nil

Fix

Initialize ret to an empty &models.ScrapedScene{} so processSceneRelationships always receives a valid pointer:

// after
ret := &models.ScrapedScene{}
if len(results) > 0 {
    ret = results[0].scrapedScene()
}
hasRelationships := s.processSceneRelationships(ctx, q, 0, ret)

This also preserves the intent of #3953: when a scraper yields no non-relationship fields but does yield relationships, the function now correctly returns a scene populated with only those relationships instead of panicking.

Panic trace

goroutine 124232 [running]:
github.com/stashapp/stash/pkg/scraper.mappedScraper.processSceneRelationships(...)
    github.com/stashapp/stash/pkg/scraper/mapped.go:89 +0xd9
github.com/stashapp/stash/pkg/scraper.mappedScraper.scrapeScene(...)
    github.com/stashapp/stash/pkg/scraper/mapped.go:191 +0x19b
...
panic in job 2 - Identifying...: runtime error: invalid memory address or nil pointer dereference

[Copilot]

Pull request overview

Fixes a panic in the mapped scraper’s scrapeScene flow by ensuring relationship processing always receives a non-nil *models.ScrapedScene, allowing “relationship-only” scraper results (per #3953) to be returned instead of crashing.

Changes:

• Initialize ret to an empty &models.ScrapedScene{} before processing scene relationships.
• Preserve existing behavior to return nil only when both direct results and relationships are absent.

Comments suppressed due to low confidence (1)

pkg/scraper/mapped.go:200

• This change fixes an important edge case (no scene fields returned but relationships are returned). There doesn't appear to be a unit test covering scrapeScene returning a non-nil scene populated only with relationships when len(results)==0; adding a regression test for this scenario would help prevent future panics/behavior regressions.

	// Initialize ret to a non-nil empty scene so that processSceneRelationships
	// can safely populate relationship fields even when no direct results were found.
	// This preserves the intent of #3953: returning a scene with only relationships.
	ret := &models.ScrapedScene{}
	if len(results) > 0 {
		ret = results[0].scrapedScene()
	}
	hasRelationships := s.processSceneRelationships(ctx, q, 0, ret)

	// #3953 - process only returns results if the non-relationship fields are
	// populated
	// only return if we have results or relationships
	if len(results) > 0 || hasRelationships {
		return ret, nil

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Gykes]

I would also recommend adding in some kind of regression test for the relationships-only scrape path.

Something that does the following:

1. configures a scene scraper with empty mappedConfig results but a populated Performers/Tags block
2. asserts the function does not panic
3. asserts the returned *ScrapedScene carries the relationships

[dutchdevil-83]

Agreed — this path is worth covering. The production fix prevents the nil dereference, but a regression test should verify that a scene scraper with no direct scene-field results still returns a non-nil ScrapedScene populated with relationship data. I’d add a test in pkg/scraper/xpath_test.go that clears the scene mappedConfig, keeps the Performers/Tags mappings, asserts scrapeScene does not panic, and verifies the returned performers/tags.