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

Exit code 0 even if fails to post comment #1187

Closed
doriandekoning opened this issue Mar 19, 2024 · 6 comments · Fixed by #1257
Closed

Exit code 0 even if fails to post comment #1187

doriandekoning opened this issue Mar 19, 2024 · 6 comments · Fixed by #1257
Milestone

Comments

@doriandekoning
Copy link

doriandekoning commented Mar 19, 2024

tfcmt version

$ tfcmt -v
tfcmt -var "target:dir" --config tfcmt.yml plan -- terraform plan

Environment

  • OS:
  • CPU Architecture:

Overview

Background:
When running tfcmt from our github actions we ocasionally hit github rate limits on the amount of comments we can post. Tfcmt then nicely logs an error about hitting the rate limit but still returns exit code 0. This can be quite dangerous if used in combination with --skip-no-changes because when the rate limit is hit no comment is posted and the user might wrongly assume there are no planned changes.

Question:
Is it intentional that when an error occurs posting a comment tfcmt exits with 0 as exit code? For example, when no github token is provided a non-zero exit code is provided but if an invalid token is provided tfcmt exits with 0.

If this is intentional it would be very useful to add an option that when set would make tfcmt fail if an error occurs.

I'd be happy to open a PR if it would be welcome!

How to reproduce

tfcmt.yaml

any

Terraform Configuration

any

Executed command and output

$ tfcmt --config tfcmt.yml plan -- terraform plan; echo "Exit code $?"
...
post a comment: POST https://api.github.com/repos/[snip]/comments: 401 Bad credentials []
Exit code 0

Debug output

$ 

Expected behaviour

Tfcmt exits with a non zero exit code if it fails to post a comment.

Actual behaviour

Tfcmt fails with a zero exit code when it fails to post a comment.

Important Factoids

No response

Note

No response

@doriandekoning doriandekoning added the bug Something isn't working label Mar 19, 2024
@suzuki-shunsuke
Copy link
Owner

Is it intentional that when an error occurs posting a comment tfcmt exits with 0 as exit code?

Yes.

This can be quite dangerous if used in combination with --skip-no-changes because when the rate limit is hit no comment is posted and the user might wrongly assume there are not planned changes.

I see. It makes sense.

I'm torn between two options.

  1. Change the exit code to 1 if it fails to post a comment
  2. Add a command line option to change the exit code

@suzuki-shunsuke
Copy link
Owner

Thank you for your feedback.

@doriandekoning
Copy link
Author

Thanks for the quick reply! Both options would work for us, the first option would be what I'd intuitively expect to happen but it might be a breaking change for some users. I'd be happy to implement and open a pr for either of the options if you like

@suzuki-shunsuke suzuki-shunsuke moved this to Todo in main Mar 19, 2024
@doriandekoning
Copy link
Author

Any updates on this @suzuki-shunsuke? I'd be happy to implement one of the options you mention but I would need to know which one you prefer :)

@suzuki-shunsuke
Copy link
Owner

Sorry for late reply.
I prefer the first option.

Change the exit code to 1 if it fails to post a comment

doriandekoning added a commit to doriandekoning/tfcmt that referenced this issue Apr 4, 2024
As discussed in this suzuki-shunsuke#1187 issue. When the github API returns a non-200 exit code (403 for bad credentials for example or when ratelimited) this is not caught and the cli does not return a non-zero exit code.
This can be dangerous in combination with the --skip-no-changes option because users might falsely assume no changes are planned.
@suzuki-shunsuke suzuki-shunsuke removed the bug Something isn't working label May 9, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in main May 9, 2024
@suzuki-shunsuke suzuki-shunsuke added this to the v4.9.1 milestone May 9, 2024
@suzuki-shunsuke
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants