Skip to content
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.

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

Download fails if the manifest, game_item.size is None #43

Closed
h143570 opened this issue Jun 21, 2021 · 8 comments
Closed

Download fails if the manifest, game_item.size is None #43

h143570 opened this issue Jun 21, 2021 · 8 comments

Comments

@h143570
Copy link

h143570 commented Jun 21, 2021

Problem

Traceback (most recent call last):
  File "C:\Users\h1435\code\gogrepoc\gogrepoc.py", line 2777, in <module>
    main(process_argv(sys.argv))
  File "C:\Users\h1435\code\gogrepoc\gogrepoc.py", line 2539, in main
    cmd_download(args.savedir, args.skipextras, args.skipids, args.dryrun, args.ids,args.os,args.lang,args.skipgalaxy,args.skipstandalone,args.skipshared, args.skipfiles,args.downloadlimit)
  File "C:\Users\h1435\code\gogrepoc\gogrepoc.py", line 1709, in cmd_download
    work_dict[dest_file] = (game_item.href, game_item.size, 0, game_item.size-1, dest_file,downloading_file)
TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'

Using the latest build from master, deleted the manifest file and regenerated it via update -full

It looks like during a mass update, GOG servers may not always produce a file size information.
It appears that while updating the local manifest with large amount of changes, like using the -full option, the size information of some files, especially in the extras section, are not always returned. This causes the size property in the manifest to be None

This causes:

work_dict[dest_file] = (game_item.href, game_item.size, 0, game_item.size-1, dest_file,downloading_file) to fail, more precisely game_item.size-1 to fail as game_item.size is None (coming from the manifest)

Manually updating that game_item via the update command appears to be refreshing the missing information and allows further processing.

Workaround attempt Fix/workaround/robustness

Setting 'game_item.size' to 0 (or any other number) in this case allows the process to continue. While in case of download there will be a message in the logs..

1:23:30 | {tom_clancys_ghost_recon}
11:23:30 |      pass       setup_tom_clancys_ghost_recon_1.4.0.0_(19232).exe
11:23:30 |      pass       gr_manual.zip
11:23:30 |      unknown    gr_command_map.zip has no size info.  skipping
11:23:30 |      pass       gr_level_builder.zip
11:23:30 |      pass       gr_wallpaper.zip
11:23:30 |      pass       gr_vehicle_tagging_guide.zip
11:23:30 |      pass       gr_avatars.zip
11:23:30 |      pass       gr_artworks.zip
11:23:30 |      pass       gr_desktop_theme.zip
11:23:30 |      pass       gr_screensaver.zip
11:23:30 | nothing to download

When the actual download starts without chunk_tree around line 1942.

if game_item.size -> end with value of -1; which will cause a HTTP 200 response without the 'Content-Range' header and produces and exception.
if game_item.size is set to a large number it will cause a pre-allocation of a large file; and it will fail the hdr format check a few lines latter.

Suggestion

Not sure if the worker code can be updated to handle this situation. Regardless we should consider adding a log message with the suggested fix (reimport the problematic item) when the problem is encountered in line 1709.

@Kalanyr
Copy link
Owner

Kalanyr commented Jun 21, 2021 via email

@h143570
Copy link
Author

h143570 commented Jun 21, 2021

Realized that a part of the report can be misunderstood, amended it.

Probably the proper thing to do is sanity check the proposed item during
the update phase and not update + give the warning/solution if it failed
the sanity check.

Detecting this problem during update could also work.

@Kalanyr
Copy link
Owner

Kalanyr commented Jun 22, 2021 via email

@Kalanyr
Copy link
Owner

Kalanyr commented Jun 22, 2021

Hmmm. Seems like the logic to handle this was already present it just only ran when a file that will end up with the same name was already present.

I've promoted it a rank, so it should catch this now.

If you have a manifest that has this issue could you try this; https://www.dropbox.com/s/c4zevl7gz2l63vf/gogrepoc.py?dl=0 and verify that it works ?

@h143570
Copy link
Author

h143570 commented Jun 22, 2021

Hmm. I do full updates reasonably regularly (1/month or two) and I haven't
encountered this but if it's a thing that happens in some locations or with
some internet connections it still needs addressing.

I also do a full update every few months and this was the first time it happened. It affected 2 extra records.

First thanks for the quick response.

Checked the manifest backup, it looks like it no longer dies.

23:40:05 | {tom_clancys_ghost_recon}
23:40:05 |      pass       setup_tom_clancys_ghost_recon_1.4.0.0_(19232).exe
23:40:05 |      pass       gr_manual.zip
23:40:05 |      unknown    gr_command_map.zip has no size info.  skipping
23:40:05 |      pass       gr_level_builder.zip
23:40:05 |      pass       gr_wallpaper.zip
23:40:05 |      pass       gr_vehicle_tagging_guide.zip
23:40:05 |      pass       gr_avatars.zip
23:40:05 |      pass       gr_artworks.zip
23:40:05 |      pass       gr_desktop_theme.zip
23:40:05 |      pass       gr_screensaver.zip

However if the manifest is large (100+) the message could be lost. Maybe adding an extra log message at the end of the command execution could help with that. Probably no need a full file list, just the message to check the logs as some files got skipped.

@Kalanyr
Copy link
Owner

Kalanyr commented Jun 23, 2021

Hmm, I think that the verify command already handles that and queueing download errors to prompt again at the end has a high chance of spamming connection timeout errors that were eventually recovered from.

@h143570
Copy link
Author

h143570 commented Jun 24, 2021

True, I probably should run the verification command more often.

@Kalanyr
Copy link
Owner

Kalanyr commented Jun 25, 2021

Gonna mark as closed, feel free to reopen if you still have an issue.

@Kalanyr Kalanyr closed this as completed Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants