Skip to content

[FrameworkBundle]  terminate with non-zero exit code when a secret could not be read #57797

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

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jul 22, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

The fix for #42038 did a bit too much. Not only did it fix the PHP error that
wasn't caught before, but also changed the exit code of the command. With this
change the PHP error will still be prevented, but the command will terminate
with a non-zero exit code to indicate the failure that occurred while reading
the stored secrets.

@carsonbot carsonbot added this to the 7.2 milestone Jul 22, 2024
@carsonbot carsonbot changed the title [FrameworkBundle] terminate with non-zero exit code when a secret could not be read [FrameworkBundle]  terminate with non-zero exit code when a secret could not be read Jul 22, 2024
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jul 22, 2024

Why? I mean, this zero-even-if-fail was explicitly desired in an earlier PR, isn't it?
I'm missing context here.

@xabbuh
Copy link
Member Author

xabbuh commented Jul 22, 2024

The current behaviour was introduced as a fix for #42038 in #43137. According to @wouterj the report wasn't about the exit code but about the PHP error that was not caught (but was also fixed in #43137).

The fix for symfony#42038 did a bit too much. Not only did it fix the PHP error that
wasn't caught before, but also changed the exit code of the command. With this
change the PHP error will still be prevented, but the command will terminate
with a non-zero exit code to indicate the failure that occurred while reading
the stored secrets.
@wouterj
Copy link
Member

wouterj commented Jul 22, 2024

Yes, I think it makes sense for this command to fail if it failed to decrypt the secrets, as also described in #57539

@chalasr
Copy link
Member

chalasr commented Jul 22, 2024

Thank you @xabbuh.

@chalasr chalasr merged commit 450113c into symfony:7.2 Jul 22, 2024
4 of 10 checks passed
@xabbuh xabbuh deleted the pr-57670 branch July 22, 2024 13:13
@fabpot fabpot mentioned this pull request Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants