Skip to content

fix: manual scraping crashing without existing item #1239

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

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

fix: manual scraping crashing without existing item #1239

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
200 changes: 102 additions & 98 deletions src/routers/secure/scrape.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ async def start_manual_session(
imdb_id: Optional[str] = None,
media_type: Optional[Literal["movie", "tv"]] = None,
magnet: Optional[str] = None,
session: Session = Depends(get_db),
) -> StartSessionResponse:
session_manager.cleanup_expired(background_tasks)

Expand Down Expand Up @@ -359,6 +360,10 @@ async def start_manual_session(

if not item:
raise HTTPException(status_code=404, detail="Item not found")

if not item_id:
item = session.merge(item)
session.commit()

container = downloader.get_instant_availability(info_hash, item.type)

Expand Down Expand Up @@ -444,7 +449,7 @@ async def manual_update_attributes(
request: Request,
session_id,
data: Union[DebridFile, ShowFileData],
session: Session = Depends(get_db),
db_session: Session = Depends(get_db),
) -> UpdateAttributesResponse:
"""
Apply selected file attributes from a scraping session to the referenced media item(s).
Expand Down Expand Up @@ -492,114 +497,113 @@ async def manual_update_attributes(
prepared_item = MediaItem(item_data)
item = next(IndexerService().run(prepared_item), None)
if item:
session.merge(item)
session.commit()
db_session.merge(item)
db_session.commit()

if not item:
raise HTTPException(status_code=404, detail="Item not found")

item = session.merge(item)
item_ids_to_submit = set()

def update_item(item: MediaItem, data: DebridFile, session: ScrapingSession):
"""
Prepare and attach a filesystem entry and stream to a MediaItem based on a selected DebridFile within a scraping session.

Cancels any running processing job for the item and resets its state; ensures there is a staging FilesystemEntry for the given file (reusing an existing entry or creating a provisional one and persisting it), clears the item's existing filesystem_entries and links the staging entry, sets the item's active_stream to the session magnet and torrent id, appends a ranked ItemStream derived from the session, and records the item's id in the module-level item_ids_to_submit set.

Parameters:
item (MediaItem): The media item to update; will be merged into the active DB session as needed.
data (DebridFile): Selected file metadata (filename, filesize, optional download_url) used to create or locate the staging entry.
session (ScrapingSession): Scraping session containing the magnet and torrent_info used to set active_stream and rank the stream.
"""
request.app.program.em.cancel_job(item.id)
item.reset()

# Ensure a staging MediaEntry exists and is linked
from sqlalchemy import select
from program.media.media_entry import MediaEntry

fs_entry = None

if item.filesystem_entry:
fs_entry = item.filesystem_entry
# Update source metadata on existing entry
fs_entry.original_filename = data.filename
else:
# Create a provisional VIRTUAL entry (download_url/provider may be filled by downloader later)
fs_entry = MediaEntry.create_virtual_entry(
original_filename=data.filename,
download_url=getattr(data, "download_url", None),
provider=None,
provider_download_id=None,
file_size=(data.filesize or 0),
)
session.add(fs_entry)
session.commit()
session.refresh(fs_entry)

# Link MediaItem to FilesystemEntry
# Clear existing entries and add the new one
item.filesystem_entries.clear()
item.filesystem_entries.append(fs_entry)
item = session.merge(item)

item.active_stream = {
"infohash": session.magnet,
"id": session.torrent_info.id,
}
torrent = rtn.rank(session.torrent_info.name, session.magnet)
item = db_session.merge(item)
item_ids_to_submit = set()

def update_item(item: MediaItem, data: DebridFile, session: ScrapingSession):
"""
Prepare and attach a filesystem entry and stream to a MediaItem based on a selected DebridFile within a scraping session.

Cancels any running processing job for the item and resets its state; ensures there is a staging FilesystemEntry for the given file (reusing an existing entry or creating a provisional one and persisting it), clears the item's existing filesystem_entries and links the staging entry, sets the item's active_stream to the session magnet and torrent id, appends a ranked ItemStream derived from the session, and records the item's id in the module-level item_ids_to_submit set.

# Ensure the item is properly attached to the session before adding streams
# This prevents SQLAlchemy warnings about detached objects
if object_session(item) is not session:
item = session.merge(item)
Parameters:
item (MediaItem): The media item to update; will be merged into the active DB session as needed.
data (DebridFile): Selected file metadata (filename, filesize, optional download_url) used to create or locate the staging entry.
session (ScrapingSession): Scraping session containing the magnet and torrent_info used to set active_stream and rank the stream.
"""
request.app.program.em.cancel_job(item.id)
item.reset()

item.streams.append(ItemStream(torrent))
item_ids_to_submit.add(item.id)
# Ensure a staging MediaEntry exists and is linked
from sqlalchemy import select
from program.media.media_entry import MediaEntry

if item.type == "movie":
update_item(item, data, session)
fs_entry = None

if item.filesystem_entry:
fs_entry = item.filesystem_entry
# Update source metadata on existing entry
fs_entry.original_filename = data.filename
else:
for season_number, episodes in data.root.items():
for episode_number, episode_data in episodes.items():
if item.type == "show":
if episode := item.get_absolute_episode(
episode_number, season_number
):
update_item(episode, episode_data, session)
else:
logger.error(
f"Failed to find episode {episode_number} for season {season_number} for {item.log_string}"
)
continue
elif item.type == "season":
if episode := item.parent.get_absolute_episode(
episode_number, season_number
):
update_item(episode, episode_data, session)
else:
logger.error(
f"Failed to find season {season_number} for {item.log_string}"
)
continue
elif item.type == "episode":
if (
season_number != item.parent.number
and episode_number != item.number
):
continue
update_item(item, episode_data, session)
break
# Create a provisional VIRTUAL entry (download_url/provider may be filled by downloader later)
fs_entry = MediaEntry.create_virtual_entry(
original_filename=data.filename,
download_url=getattr(data, "download_url", None),
provider=None,
provider_download_id=None,
file_size=(data.filesize or 0),
)
db_session.add(fs_entry)
db_session.commit()
db_session.refresh(fs_entry)

Comment on lines +535 to +545
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep manual update atomic

Calling db_session.commit() inside update_item finalizes part of the transaction before the rest of the loop finishes. If any later step raises, the request returns 500 but prior commits have already persisted partial state. Flush instead so the primary key is available without breaking atomicity.

             db_session.add(fs_entry)
-            db_session.commit()
+            db_session.flush()
             db_session.refresh(fs_entry)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fs_entry = MediaEntry.create_virtual_entry(
original_filename=data.filename,
download_url=getattr(data, "download_url", None),
provider=None,
provider_download_id=None,
file_size=(data.filesize or 0),
)
db_session.add(fs_entry)
db_session.commit()
db_session.refresh(fs_entry)
fs_entry = MediaEntry.create_virtual_entry(
original_filename=data.filename,
download_url=getattr(data, "download_url", None),
provider=None,
provider_download_id=None,
file_size=(data.filesize or 0),
)
db_session.add(fs_entry)
db_session.flush()
db_session.refresh(fs_entry)
🤖 Prompt for AI Agents 
In src/routers/secure/scrape.py around lines 535 to 545, the code calls
db_session.commit() while inside update_item which finalizes part of the
transaction early; replace that commit with db_session.flush() so the new
fs_entry gets a primary key available for subsequent operations but the overall
transaction remains atomic (you can keep or call db_session.refresh(fs_entry)
after the flush if you need loaded DB defaults), and ensure the actual commit is
performed once at the end of the outer operation rather than here.

# Link MediaItem to FilesystemEntry
# Clear existing entries and add the new one
item.filesystem_entries.clear()
item.filesystem_entries.append(fs_entry)
item = db_session.merge(item)

item.active_stream = {
"infohash": session.magnet,
"id": session.torrent_info.id,
}
torrent = rtn.rank(session.torrent_info.name, session.magnet)

# Ensure the item is properly attached to the session before adding streams
# This prevents SQLAlchemy warnings about detached objects
if object_session(item) is not db_session:
item = db_session.merge(item)

item.streams.append(ItemStream(torrent))
item_ids_to_submit.add(item.id)

if item.type == "movie":
update_item(item, data, session)
else:
for season_number, episodes in data.root.items():
for episode_number, episode_data in episodes.items():
if item.type == "show":
if episode := item.get_absolute_episode(
episode_number, season_number
):
update_item(episode, episode_data, session)
else:
logger.error(f"Failed to find item type for {item.log_string}")
logger.error(
f"Failed to find episode {episode_number} for season {season_number} for {item.log_string}"
)
continue

item.store_state()
log_string = item.log_string
session.merge(item)
session.commit()
elif item.type == "season":
if episode := item.parent.get_absolute_episode(
episode_number, season_number
):
update_item(episode, episode_data, session)
else:
logger.error(
f"Failed to find season {season_number} for {item.log_string}"
)
continue
elif item.type == "episode":
if (
season_number != item.parent.number
and episode_number != item.number
):
continue
update_item(item, episode_data, session)
break
Comment on lines +592 to +598
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix the episode matching guard.

The guard uses and, so we still update the current episode whenever either the season or the episode number happens to match. That attaches the wrong file (e.g., same season, different episode). Use or so we skip as soon as either component diverges.

-                    if (
-                        season_number != item.parent.number
-                        and episode_number != item.number
-                    ):
+                    if (
+                        season_number != item.parent.number
+                        or episode_number != item.number
+                    ):
                         continue
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (
season_number != item.parent.number
and episode_number != item.number
):
continue
update_item(item, episode_data, session)
break
if (
season_number != item.parent.number
or episode_number != item.number
):
continue
update_item(item, episode_data, session)
break
🤖 Prompt for AI Agents 
In src/routers/secure/scrape.py around lines 592 to 598, the episode matching
guard currently uses AND which allows a match when either season or episode
number matches; change the condition to use OR so that if either the
season_number differs from item.parent.number or the episode_number differs from
item.number we continue (i.e., skip this item) and only call update_item when
both components match.

Comment on lines +592 to +598
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix episode guard condition

With the current and check, any payload that matches the season but not the episode (or vice‑versa) will slip through and overwrite the wrong record. Switch to or (or invert the condition) so we only update when both numbers match, preventing cross‑episode data corruption.

-        if (
-            season_number != item.parent.number
-            and episode_number != item.number
-        ):
+        if (
+            season_number != item.parent.number
+            or episode_number != item.number
+        ):
🤖 Prompt for AI Agents 
In src/routers/secure/scrape.py around lines 592 to 598, the guard uses an "and"
so a mismatch on one field can still pass; change the condition to continue when
either the season or episode mismatches (i.e., use "or" between the two
inequality checks or invert the logic so you only proceed when both
season_number == item.parent.number AND episode_number == item.number) so
update_item is only called when both numbers match.

else:
logger.error(f"Failed to find item type for {item.log_string}")
continue

item.store_state()
log_string = item.log_string
db_session.merge(item)
db_session.commit()

# Sync VFS to reflect any deleted/updated entries
# Must happen AFTER commit so the database reflects the changes
Expand Down