Download fails if the manifest, game_item.size is None

[kalanyr]

Download fails if the manifest, game_item.size is None

[Kalanyr]

I've never had this error happen but I also stop updates ASAP if I see Sales are making things unstable. 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.

…

On Tue, 22 Jun 2021, 04:58 Balazs Bessenyei, ***@***.***> wrote: Traceback (most recent call last): File "C:\Users\h1435\code\gogrepoc\gogrepoc.py", line 2777, in 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. 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. 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. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#43>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKZ334ZCZ6C3UNCWEJETS3TT6DTRANCNFSM47CETTBA> .

[h143570]

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]

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. The size in manifest important for verification both pre and post download so just replacing the None with an int isn't an option. Sanity check is probably the best long term option but going to have to think about it a bit (eg need to distinguish between not yet uploaded items which should be known and bad items) I'll have a look in the next couple of days if I can at least easily gracefully skip a download when manifest size is None and spit a proper warning to recupdate the specific id.

…

On Tue, 22 Jun 2021, 06:25 Balazs Bessenyei, ***@***.***> wrote: 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. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#43 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKZ336SDHHV2NOLUEXLTHTTT6N4HANCNFSM47CETTBA> .

[Kalanyr]

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]

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]

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]

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

[Kalanyr]

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