search

sct / overseerr

Request management and media discovery tool for the Plex ecosystem
https://overseerr.dev
MIT License
3.72k stars 435 forks source link

Availability sync fails to correctly identify removed media items #3708

Open epitsi opened 9 months ago

epitsi commented 9 months ago

Description

This report is the level-down of the issue that I originally reported here: https://github.com/sct/overseerr/pull/3219#issuecomment-1605803382

What seems to be happening is that Overseerr stores an externalServiceId as part of the media metadata, which presumably is the id of that movie in Radarr/Sonarr at the time that the entry was created. That id is subsequently used during availability sync to get the availability status of that item.

This is however problematic, as the ids assigned by Radarr/Sonarr are not stable. In other words, id 0 can be a different media entry in 2 separate instances of each app. The current implementation of Overseerr assumes that externalServiceId will always point to the media it was originally assigned to. Which in turns means that when Overseerr checks for the availability of a specific media item using externalServiceId, it is possible that the response does not correspond to the intended media item.

There is an easy way around this problem as the v3 API allows querying for movies by tmdbId and TV series by tvdbId. Which means there is no reason to rely on the error-prone externalServiceId.

Version

1.33.2

Steps to Reproduce

  1. Create a movie media entry in the local Overseer database with an externalServiceId that points to any random existing item in Radarr; set the item's status to AVAILABLE (5).
  2. Run availability sync and verify that the movie's status is not changed to UNKNOWN, despite the fact that the specific tmdbId entry does not exist in Radarr.

(similar for Sonarr)

Screenshots

No response

Logs

No response

Platform

desktop

Device

Macbook Pro 14

Operating System

Sonoma 14.1.2

Browser

Chrome

Additional Context

Code of Conduct

epitsi commented 9 months ago

I have a working implementation (very small PR) for fixing this for movies. Will expand tomorrow to include TV shows as well.

OwsleyJr commented 9 months ago

Can you give me a case when the externalServiceId will not match the correct media item? If the id does not match, the Radarr/Sonarr scanner should update the id to the correct one.

epitsi commented 9 months ago

What I can confidently say is that my current Overseer & Radarr configs, have numerous instances of this issue. That's how I was able to debug it: by pointing my dev environment to my own "prod" Overseer config, and then trying to see why availability sync was not working as intended.

If I had to try and reconstruct how I got in this situation, I believe it went as follows: I used to have 2 instances of Radarr connected to Overseer. That was a few years ago. Eventually, I deleted one of those instances and all of its content, but the Overseer instance and its database has remained the same over the years. This was well before availability sync was introduced.

I am now in a situation where I can clearly see that for Overseerr pulls movie A from its database, and goes out to Radarr with externalServiceId X, looking for the movie. Radarr responds successfully with a movie. Problem is, the tmdbId of what Overseerr was looking for and Radarr returned, do not match. What I believe is happening is that externalServiceId was valid in the old Radarr instance that was removed, and in the new instance it just maps to a random entry. If I was luckier, it would have pointed to nothing in the new instance, at which point the movie would have been deleted by Overseerr.

Two quick thoughts:

  1. Why must you use externalServiceId if tmdbId lookups are available? The latter seems like a much safer option than the former?
  2. An alternative solution would be for Overseerr to validate that the response it receives for a certain externalServiceId matches the tmdbId that it itself holds. AFAICT, there is no mechanism that does that right now. Availability sync definitely doesn't (that part code is simple enough for me to say that). I don't know if there is anything else in the code that is supposed to do that (I'd love a pointer to a file if you have one). But I can tell you for sure that it doesn't work, given my own issues that have led me getting into development here.
OwsleyJr commented 9 months ago

What I can confidently say is that my current Overseer & Radarr configs, have numerous instances of this issue. That's how I was able to debug it: by pointing my dev environment to my own "prod" Overseer config, and then trying to see why availability sync was not working as intended.

If I had to try and reconstruct how I got in this situation, I believe it went as follows: I used to have 2 instances of Radarr connected to Overseer. That was a few years ago. Eventually, I deleted one of those instances and all of its content, but the Overseer instance and its database has remained the same over the years. This was well before availability sync was introduced.

I am now in a situation where I can clearly see that for Overseerr pulls movie A from its database, and goes out to Radarr with externalServiceId X, looking for the movie. Radarr responds successfully with a movie. Problem is, the tmdbId of what Overseerr was looking for and Radarr returned, do not match. What I believe is happening is that externalServiceId was valid in the old Radarr instance that was removed, and in the new instance it just maps to a random entry. If I was luckier, it would have pointed to nothing in the new instance, at which point the movie would have been deleted by Overseerr.

Two quick thoughts:

  1. Why must you use externalServiceId if tmdbId lookups are available? The latter seems like a much safer option than the former?
  2. An alternative solution would be for Overseerr to validate that the response it receives for a certain externalServiceId matches the tmdbId that it itself holds. AFAICT, there is no mechanism that does that right now. Availability sync definitely doesn't (that part code is simple enough for me to say that). I don't know if there is anything else in the code that is supposed to do that (I'd love a pointer to a file if you have one). But I can tell you for sure that it doesn't work, given my own issues that have led me getting into development here.

Hmm, I think there was a reason in the original development that we moved from the tmdbId. But I don't think it should be an issue now hopefully. We can definitely try it though and see how it goes.

stale[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.