Skip to content

Fix binary issue file handling - #3514

Open
Sanjays2402 wants to merge 2 commits into
mozilla:masterfrom
Sanjays2402:fix/ignore-binary-issue-files
Open

Fix binary issue file handling#3514
Sanjays2402 wants to merge 2 commits into
mozilla:masterfrom
Sanjays2402:fix/ignore-binary-issue-files

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #3365. Binary files referenced by analysis issues could raise UnicodeDecodeError while building issue hashes and abort publication. Treat undecodable files like missing files so those issues are skipped, with a regression covering the WebP failure.

Skip files that cannot be decoded while building issue hashes so binary artifacts cannot abort publication. Add a regression for the reported WebP failure.
Comment thread bot/code_review_bot/revisions/base.py Outdated
except (FileNotFoundError, IsADirectoryError):
logger.warning("Failed to find issue's related file", path=file_path)
except (FileNotFoundError, IsADirectoryError, UnicodeDecodeError):
logger.warning("Failed to read issue's related file", path=file_path)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a separate except clause for this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split out in c620ae3. UnicodeDecodeError now has its own clause with a message that
says what actually went wrong, so the "failed to find" warning stays about missing paths:

except (FileNotFoundError, IsADirectoryError):
    logger.warning("Failed to find issue's related file", path=file_path)
    file_content = None
except UnicodeDecodeError:
    logger.warning(
        "Failed to decode issue's related file, it is not text",
        path=file_path,
    )
    file_content = None

bot/tests/test_revisions.py is 6 passed with test_binary_file_content_is_ignored
green; test_revision_before_after fails the same way on the unmodified branch
(TypeError: 'NoneType' object does not support item assignment at line 234), so it is
not from this change.

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

Successfully merging this pull request may close these issues.

WebP artifact file must be excluded from artifacts and not attempted to be parse as issues file

2 participants