--continue-on-errors (skip "bad" APKs)

[obfusk]

[IzzySoft]

• optionally consider "quarantining" bad APKs?
• other errors could include APKs without metadata, metadata without APKs, "invalid" metadata (missing fields, unknown fields, broken YAML), broken images or unknown image formats (icon, screenshots etc).
• at some later point maybe even things like "hits" from scanners.

[obfusk]

I implemented --continue-on-errors but currently only for APKs (metadata YAML should always be valid). Images etc. aren't inspected or processed by any of the code, just added to the index, so no way to know if they are valid (modulo encoding errors when reading the .txt files I guess). Keeping this open for now.

[obfusk]

Quarantining bad APKs is up to the user/hooks for now. You can use errors.json for that.

[obfusk]

If anyone wants more comprehensive checks on the data before index generation they should write a hook :)

[obfusk]

APKs without metadata, metadata without APKs

These are both errors, and become warnings with --continue-on-errors.

[IzzySoft]

Full ack: with the "handler" being there, and hooks possible, it's sufficient the handler triggers and one can "hook in" what should happen (quarantining, deleting, disabling updates – whatever the maintainer(s) need). Thanks!

metadata YAML should always be valid

Agreed, that's what linting is for. So crashing on invalid YAML is… well, deserved (with or without the comma even – and yeah, I know I get hit by that from time to time too 🙈)

You can use errors.json for that.

That's something I love specifically. This way even those who cannot write their own hooks can easily catch and process errors – like (pseudo-code): "if file_exists(error.json) && !empty(error.json) { for apk in errors … }" No need to evaluate console output (STDERR/STDOUT) for that 🤗 The text in the "error" should be easy enough to parse.

errors.json will be replaced with each run I guess (not appended to)?

[obfusk]

errors.json will be replaced with each run I guess (not appended to)?

As I did not want to replace existing errors that may or may not have been handled, it appends. If you use --continue-on-errors, you are responsible for handling errors.json and cleaning it up when you're done. Every entry has a timestamp, which will be the same for every APK from that update run and the same timestamp used for the index etc. Unlike fdroidserver timestamps reflect when apkrepotool update was called and are not different depending on what order APKs happen to be processed in.

[obfusk]

Currently, errors.json is always generated/updated, even when --continue-on-errors it not used (but will always be {} as nothing will be added without that option).

[IzzySoft]

As I did not want to replace existing errors that may or may not have been handled, it appends.

Makes sense. And ack for who's responsible.

always generated/updated

OK, that means a different checking only: not "if file exists and is not empty", but "if file exists and not an empty JSON object". So then dealing with the issues and deleting errors.json, an empty one would be generated on the next run (if successful; else a non-empty one of course).

Thanks! That's also a clear state to handle. I guess it would make sense if without --continue-on-errors, the error causing processing to halt would also be logged in errors.json? So one knows what to deal with.

[obfusk]

I guess it would make sense if without --continue-on-errors, the error causing processing to halt would also be logged in errors.json? So one knows what to deal with.

Not really? Without it, apkrepotool update exits with an error message. Not like you need to grep any log files. And the whole point of having --continue-on-errors and errors.json is to keep going and allow automatically handling those APKs instead of failing and requiring manual intervention.

[obfusk]

I changed it so that errors.json will not be loaded or saved without --continue-on-errors. I don't think it makes sense to create a file with an empty object without that option used.

I don't think it would make sense to add a single fatal error to that file but ignore any other APKs that would also have had errors. I guess we could have a third mode where it continues and writes to the JSON file but still exits with non-zero if any errors were encountered, if there's a need for that.

[IzzySoft]

That sounds good, yeah – thanks! And that "third mode", too. Ideally not just 0/1 but dedicated exit codes, easier for scripting error handling probably. Can't hurt at least…