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

Request for improved handling of decode errors during builds. #370

Closed
Cptmorgan27 opened this issue Oct 5, 2022 · 10 comments
Closed

Request for improved handling of decode errors during builds. #370

Cptmorgan27 opened this issue Oct 5, 2022 · 10 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Cptmorgan27
Copy link

Cptmorgan27 commented Oct 5, 2022

Hi,

Recently, my team encountered an error during a docker build where we hit the docker build log limit, and the output was clipped; this resulted in a random non-utf-8 character being produced. The log limit non-deterministically clipped off content in the middle of a UTF-8 Character. In our logs, there was a UTF-8 Character ➤ present; this is made up of 3 bytes (see Find all Unicode Characters from Hieroglyphs to Dingbats – Unicode Compart ) 0xE2 0x9E 0xA4. the 1MiB log clipping terminates the output at 0xE2 0x9E. It took us some time and troubleshooting to come to this finding.

We are asking if there could be more graceful handling of these decoding errors implemented as we believe that the current handling covered up the log limit error and produced a stack trace with the not-so-helpful UnicodeDecodeError: 'utf-8' codec can't decode bytes in position 10-11: invalid continuation byte

Thank you!

@gabrieldemarmiesse
Copy link
Owner

Thank you for the issue! Can you tell me how you called buildx exactly? which arguments in the function were used I mean. I'm trying to understand better the situation. Also since it happened to you, what do you think would be a good error message to display to the user in this situation? Thanks!

@LewisGaul
Copy link
Collaborator

It might be better to use one of the other decode error handling options, e.g. ignore errors instead of raising https://docs.python.org/3/library/codecs.html#error-handlers. Or could explicitly handle cases where decode errors are 'expected' by checking it occurs at the end of the string and just trimming the end off.

@Cptmorgan27
Copy link
Author

Cptmorgan27 commented Oct 5, 2022

We were calling python_on_whales.docker.build as such

python_on_whales.docker.build(
                context_path=Path.cwd(),
                secrets=secrets,
                stream_logs=True,
                **docker_build_params.get_denormalized_params(...)
               )

Note get_denormalized_params() is a function we have that maps common build params for python_on_whales.docker.build or docker-py.build, which ever is being used; this is in a common use internal library we have.

We are currently working on adding builder="larger_log" to the function call to help with the build log limit.

But yes, something along @LewisGaul comment would do. Mainly a way that allows for the docker build error `[output clipped, log limit 1MiB reached] to be raised to the user.

@gabrieldemarmiesse
Copy link
Owner

Thank you for the feedback! It's much clearer. A pull request is welcomed if someone is motivated, otherwise I'll implement it when I have the time

@gabrieldemarmiesse gabrieldemarmiesse added bug Something isn't working help wanted Extra attention is needed labels Oct 9, 2022
magichair added a commit to magichair/python-on-whales that referenced this issue Oct 10, 2022
@Cptmorgan27
Copy link
Author

@gabrieldemarmiesse my colleague has opened a PR to address this issue please review at your earliest convenience.

@gabrieldemarmiesse
Copy link
Owner

Great, thank you :)

@magichair
Copy link
Contributor

@gabrieldemarmiesse Happy to get a new release cut whenever you're ready and we can test it out over here. 🙏 Thank you.

@gabrieldemarmiesse
Copy link
Owner

I'm planning to do it on Thursday :)

@gabrieldemarmiesse
Copy link
Owner

The new release is here, can you try it and tell me if that works for you? If yes, then we can close this issue

@gabrieldemarmiesse
Copy link
Owner

closing this issue as it seems resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants