search

alexta69 / metube

Self-hosted YouTube downloader (web UI for youtube-dl / yt-dlp)
GNU Affero General Public License v3.0
4k stars 263 forks source link

MeTube Docker version fails to delete files. #264

Closed pakkux closed 1 year ago

pakkux commented 1 year ago

Hey!

I am using MeTube Docker. I added env: DELETE_FILE_ON_TRASHCAN = true So now when I try to remove completed it fails: Traceback (most recent call last): File "/usr/local/lib/python3.8/site-packages/aiohttp/web_protocol.py", line 433, in _handle_request resp = await request_handler(request) File "/usr/local/lib/python3.8/site-packages/aiohttp/web_app.py", line 504, in _handle resp = await handler(request) File "app/main.py", line 112, in delete status = await (dqueue.cancel(ids) if where == 'queue' else dqueue.clear(ids)) File "/app/app/ytdl.py", line 303, in clear os.remove(os.path.join(dl.download_dir, dl.info.filename)) File "/usr/local/lib/python3.8/posixpath.py", line 76, in join a = os.fspath(a) TypeError: expected str, bytes or os.PathLike object, not NoneType

I did some debugging and it seems that ytdl.py on line 302: os.remove(os.path.join(dl.download_dir, dl.info.filename)) there is no value in dl.download_dir. I did try and replaced the line with: os.remove(os.path.join(self.config.DOWNLOAD_DIR, dl.info.filename)) and with that it does work. I am no python developer so that was the fastest fix I could think of. Could anyone with proper skill take a look into it and look why dl.download_dir is null?

o0laxkilla0o commented 1 year ago

I also am coming across this issue on a newly deployed instance

PikuZheng commented 1 year ago

It looks like this happens when a subfolder is manually selected when downloading. Does this issue also exist if you download directly?

pakkux commented 1 year ago

Downloading and viewing in browser is working.

EDIT: I will try to debug it bit more this weekend. Maybe I can discover something new.

guahki commented 1 year ago

I introduced the DELETE_FILE_ON_TRASHCAN option and myself realized, that is far from perfect.

It seems like the dl.download_dir is not stored in the state file and thus lost when the server is restarted.

I also had problems with failed downloads.

Right now I opened https://github.com/alexta69/metube/pull/284, which "solves" the issue by just not deleting the file but going on with logging a warning and deleting the download from the "Completed" list without deleting the file. Maybe @alexta69 can give some input, if/how to get the full filepath from the dl.info object.

alexta69 commented 1 year ago

No idea on the dl.info subject. But the dl.download_dir can be added to the state if it's needed. (Of course with care, to remain backwards compatible, i.e. allow None value there.)

I'll merge the PR in the meantime, because the graceful failure is better than the current situation, but I hope we can fix it more thoroughly.

alexta69 commented 1 year ago

Can #285 be related to this?