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

Bug - Checks containing a lot of logs/data are not sent to GitHub #131

Closed
j-martin opened this issue Feb 24, 2021 · 10 comments · Fixed by #138
Closed

Bug - Checks containing a lot of logs/data are not sent to GitHub #131

j-martin opened this issue Feb 24, 2021 · 10 comments · Fixed by #138
Labels
bug Something isn't working

Comments

@j-martin
Copy link

j-martin commented Feb 24, 2021

Problem

Since 1.10 https://github.com/jenkinsci/github-checks-plugin/pull/101/files the plugin errors out if the check payload is too large. I am assuming that prior to 1.10 GitHub would reject the request on their side.

Unfortunately, some of our checks contain a massive amount of logs which means that the checks will never be sent to GitHub. This is an issue for us as it may prevent people from merging to a protected branch. Our workaround is to only depend on the main GitHub check (not published by this plugin) that does not contain any logs.

Expected behavior

A truncated version is sent to GitHub. Alternatively, skip all logs altogether, but it would be nice to keep it the payload is under 65335 bytes.

Actual behavior

The check result is not posted to GitHub.

@timja
Copy link
Member

timja commented Feb 24, 2021

cc @mrginglymus

@j-martin
Copy link
Author

I think that's what https://github.com/jenkinsci/github-checks-plugin/pull/101/files is supposed to do. It might just be that the max size be lowered to allow the rest of the payload to be sent.

https://github.com/jenkinsci/github-checks-plugin/pull/101/files#diff-fd0d45e50475b17adec8ca170773d293c9de8536261d66d1f1484598723a862cR37

I am guessing saying it is specific to 1.10 might be wrong.

This is the error we are seeing in our logs so I guess it is still coming from GitHub:

[GitHub Checks] Failed Publishing GitHub checks: org.kohsuke.github.GHFileNotFoundException: https://api.github.com/repos/alloytech/alloy/check-runs/1961940126 {"message":"Validation Failed","errors":[{"resource":"CheckRun","code":"custom","field":"summary","message":"summary exceeds a maximum bytesize of 65535"}],"documentation_url":"https://docs.github.com/rest/reference/checks#update-a-check-run"}

@mrginglymus
Copy link
Contributor

What plugin is sending these checks, or is it a manual invocation of the publishChecks step?

@KalleOlaviNiemitalo
Copy link

The GitHub error response says "a maximum bytesize of 65535" but TruncatedString counts characters rather than bytes.

@KalleOlaviNiemitalo
Copy link

https://git.luolix.topmunity/t/undocumented-65535-character-limit-on-requests/117564 likewise says the limit is 65535 bytes, and applies to each property separately.

I hope that encoding a newline as \n in a JSON string won't consume two of those bytes.

@KalleOlaviNiemitalo
Copy link

From jenkinsci/junit-plugin#192 (comment), GitLab and Bitbucket Server have limits given in characters rather than bytes. It might be best to make the Checks API plugin provide an array of strings and make the GitHub Checks plugin measure how many bytes each string will cost.

@mrginglymus
Copy link
Contributor

I can see two things to do - we can update TruncatedString to truncate on byte-length rather than char-length. If that's the root of this problem, then we'll solve it without having to update anything else.

The other thing is to make the TruncatedString able to truncate on char or bytes at the callers discretion; as there's only one at the moment it's less urgent to implement that.

@timja
Copy link
Member

timja commented Feb 24, 2021

The other thing is to make the TruncatedString able to truncate on char or bytes at the callers discretion; as there's only one at the moment it's less urgent to implement that.

not published yet but this was just hosted:
https://github.com/jenkinsci/gitea-checks-plugin

@mrginglymus
Copy link
Contributor

I'm doing it now, and it's easy enough to do both. PR incoming...

@mrginglymus
Copy link
Contributor

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

Successfully merging a pull request may close this issue.

5 participants