sentriz / gonic

music streaming server / free-software subsonic server API implementation
ircs://irc.libera.chat/#gonic
GNU General Public License v3.0
1.5k stars 105 forks source link

Panic on migration of podcast paths -> PEs need generated filenames #393

Closed brian-doherty closed 8 months ago

brian-doherty commented 8 months ago

gonic version:

if from docker, docker tag: if from source, git tag/branch: 69c02e8352c10276697184d3c24d0f3253ec8c4d

I'm seeing a panic when migrating to the latest version:

goroutine 1 [running]: log.Panicf({0xc2a591?, 0xc00002b968?}, {0xc000433c20?, 0xc00002b8a8?, 0x12?}) /opt/go/src/log/log.go:439 +0x65 main.main() /home/brian/gonic/cmd/gonic/gonic.go:153 +0x10fa 2023/10/18 11:51:40 error migrating database: "202309161411": find old podcast episode path: stat /media/media/Podcasts/HeadsInTheCloud/default_tc.mp3: no such file or directory panic: error migrating database: "202309161411": find old podcast episode path: stat /media/media/Podcasts/HeadsInTheCloud/default_tc.mp3: no such file or directory

The root cause is that:

1) Some podcasts use the same filename for each episode with ? parameters to differentiate. Example: https://feeds.simplecast.com/l2i9YnTd 2) This leads to gonic using the same filename for different episodes. In addition to the obvious problem (each episode is actually the latest episode when you play it), this leads the podcast purger to delete that file which leaves future episodes pointing to a non-existing file.

I suggest two changes to fix this issue:

1) The key change to make is to start creating a unique local filename for podcast episodes independent of what's in the RSS, using mktemp() or equivalent. 2) I would also suggest not panicking on that migration just because a podcast episode filename is missing.

sentriz commented 8 months ago
  1. i think this should already be the case with the new fileutil.Unique() function
  2. i agree :+1: i just pushed a commit for that. could you try fetch and re-run? i think it should retry the failed migration since it didn't finish last time

cheers :+1:

brian-doherty commented 8 months ago

That did it. Thanks! I will check on 1 by re-fetching the podcasts.

B

On Wed, Oct 18, 2023 at 2:11 PM Senan Kelly @.***> wrote:

  1. i think this should already be the case with the new fileutil.Unique() function
  2. i agree 👍 i just pushed a commit for that. could you try fetch and re-run? i think it should retry the failed migration since it didn't finish last time

cheers 👍

— Reply to this email directly, view it on GitHub https://github.com/sentriz/gonic/issues/393#issuecomment-1769162307, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASFD42LHW6DGMVT5MM7M6VDYAASVTAVCNFSM6AAAAAA6F23PX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONRZGE3DEMZQG4 . You are receiving this because you authored the thread.Message ID: @.***>

sentriz commented 8 months ago

how did it go with 1.?

brian-doherty commented 8 months ago

Yeah, it went fine. I just hadn't updated in a while :D

B

On Fri, Oct 20, 2023 at 5:45 AM Senan Kelly @.***> wrote:

how did it go with 1.?

— Reply to this email directly, view it on GitHub https://github.com/sentriz/gonic/issues/393#issuecomment-1772511323, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASFD42NZR3ECSVKLZ47CKDLYAJI3TAVCNFSM6AAAAAA6F23PX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGUYTCMZSGM . You are receiving this because you authored the thread.Message ID: @.***>

sentriz commented 8 months ago

nice, thanks for testing :+1: