Skip to content

Add error handling for sgx ci #4222

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lum1n0us
Copy link
Collaborator

Process completed with exit code 143.

It is a known issue with GitHub-hosted runners. Usually, increasing the swap file can help avoid it. However, sometimes error 143 still occurs. To prevent confusion, let's capture error 143 and allow the CI to pass.

@lum1n0us lum1n0us marked this pull request as draft April 27, 2025 06:42
@lum1n0us lum1n0us force-pushed the fix/capture_ci_143_code branch from 659d11d to a2c5637 Compare April 27, 2025 06:53
> Process completed with exit code 143.

It is a known issue with GitHub-hosted runners. Usually, increasing the swap
file can help avoid it. However, sometimes error 143 still occurs. To prevent
confusion, let's capture error 143 and allow the CI to pass.
@lum1n0us lum1n0us force-pushed the fix/capture_ci_143_code branch from a2c5637 to ec65023 Compare April 27, 2025 07:26
@lum1n0us lum1n0us marked this pull request as ready for review April 27, 2025 07:26
Copy link
Collaborator

@TianlongLiang TianlongLiang left a comment

Choose a reason for hiding this comment

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

LGTM

if [ $exitcode -eq 143 ]; then
echo "$exitcode is a known GitHub-hosted runner issue"
echo "::notice::Bypass an error with code 143 in SGX CI"
exit 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't this risk us to merge broken changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe it will not introduce any other unexpected breaking changes.

  • Only bypass exit code 143. Other non-zero codes will still be captured.
  • Spec_test scripts, all.py and runtest.py, will clamp return codes, and 143 is not one of them.
  • 143 is not an errno in errno.h

Copy link
Collaborator

Choose a reason for hiding this comment

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

well, my concern is:

  1. someone submits a PR with broken changes
  2. the ci happened to pass because of this workaround
  3. reviewers don't notice it and merge

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Based on my observations, if there is an error from the spec-test itself, 143 won't be generated. I believe the 143 code will not cover other errors. Those accidental 143 errors are quite annoying, so I want to relieve our daily work. If directly bypassing is too aggressive, how about automatically rerunning if it fails?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Those accidental 143 errors are quite annoying, so I want to relieve our daily work.

i agree.

If directly bypassing is too aggressive, how about automatically rerunning if it fails?

it seems nicer.

@yamt
Copy link
Collaborator

yamt commented May 1, 2025

It is a known issue with GitHub-hosted runners. Usually, increasing the swap file can help avoid it.

do you have a reference to a bug ticket or something?

Copy link
Collaborator

@xujuntwt95329 xujuntwt95329 left a comment

Choose a reason for hiding this comment

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

LGTM

@TianlongLiang
Copy link
Collaborator

It is a known issue with GitHub-hosted runners. Usually, increasing the swap file can help avoid it.

do you have a reference to a bug ticket or something?

actions/runner-images#6680, there is also an approach we can try: adding -large suffix to ubuntu runner(runs-on: ubuntu-latest-large //THIS LINE HERE)

@yamt
Copy link
Collaborator

yamt commented May 8, 2025

It is a known issue with GitHub-hosted runners. Usually, increasing the swap file can help avoid it.

do you have a reference to a bug ticket or something?

actions/runner-images#6680, there is also an approach we can try: adding -large suffix to ubuntu runner(runs-on: ubuntu-latest-large //THIS LINE HERE)

large runners require some extra money, don't they?

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.

4 participants