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

Attach some debug info to release runner errors #79569

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

moxian
Copy link
Contributor

@moxian moxian commented Feb 6, 2025

Summary

None

Purpose of change

Currently our release notes are broken (last good: cdda-experimental-2025-02-05-1708, first bad: cdda-experimental-2025-02-06-0250) for no apparent reason.

The runner logs are

Run npm install @actions/github
  npm install @actions/github
  node build-scripts/generate-release-notes.js '***' 'cdda-experimental-2025-02-06-0250' "$(git log -1 --format='%H')" > notes.txt
  shell: /usr/bin/bash -e {0}
  env:
    GITHUB_TOKEN: ***
    REPOSITORY_NAME: Cataclysm-DDA

added 24 packages in 1s
Failed generating release notes with error: HttpError

unenlightening.

Somehow, the release notes work fine in my own fork (https://github.com/moxian/Cataclysm-DDA/releases)
(And also works fine locally. And also when i hackishly try it in a pull request )

Describe the solution

Add a tiny bit of context to the js errors. This does nothing to fix the issues, of course, but I have tepid hope that it might bring some light on what's going on at least

Describe alternatives you've considered

Idk, figure out what's happening and fix the root cause in one go.
Also, the classic: not do this.
I tried putting the context into console.log() but that one doesn't make its way into the CI log.
I guess i could write

.catch((e) => { console.error("context here") ; throw e; } )

instead of what I'm doing right now, I don't think either is much better than the other. Lmk if you have strong opinions here.

Testing

Tried breaking it in a couple of ways. Got reasonably readable error out each time (
example - https://github.com/moxian/Cataclysm-DDA/actions/runs/13189583076/job/36819655541?pr=19
anoter example from an earlier version of the code - https://github.com/moxian/Cataclysm-DDA/actions/runs/13189397885/job/36819126328
Thinking more about it, I never got a literal HttpError we are seeing in CI right now, it has always been HttpError: Something something.

Additional context

The /releases/generate-notes endpoint seems to require write permissions to the repo so I cannot really test that one locally for CleverRaven. (specifically the docs say it requires "Contents" repository permissions (write) for "fine-grained tokens". I'm not sure what is the implication on the github actions runners).

I suspect the fix would be adding

permissions:
  - contents: write

to the yaml edit: If this is a new change that's github is slowly rolling out. And I can PR that. But idk if that's neccessary, and I'd much prefer to leave perms at default if that's an option...
Late edit: nevermind, we already have contents: write permissions, as seen in Set up job section of the runner log

@github-actions github-actions bot added Code: Build Issues regarding different builds and build environments json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Feb 6, 2025
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Feb 7, 2025
@akrieger
Copy link
Member

akrieger commented Feb 7, 2025

Lets hope the data doesn't contain our secrets lol.

@moxian
Copy link
Contributor Author

moxian commented Feb 7, 2025

Github says that they find-and-replace the secrets in the logs so as log as we don't mangle the presentation too much (e.g. by hex-encoding or putting the data through base64), the censoring machinery should do its job fine.

@moxian
Copy link
Contributor Author

moxian commented Feb 7, 2025

By the way, update, the release notes are back to working fine - see https://github.com/CleverRaven/Cataclysm-DDA/releases/tag/cdda-experimental-2025-02-07-0026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Build Issues regarding different builds and build environments json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants