chore(ci): add feat/design-refresh branch for internal distribution#2504
chore(ci): add feat/design-refresh branch for internal distribution#2504
Conversation
In the `distribute_internal` and `distribute_external` workflows, the `determine_platforms` job would fail on manual runs when a single platform was selected. This was due to the shell command exiting with a non-zero status code if a condition was false. By appending `|| true`, the commands now always exit successfully, ensuring the workflow proceeds correctly regardless of the platform selection.
📝 WalkthroughWalkthroughTwo GitHub Actions workflow files are updated to add error handling safeguards ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/distribute_internal.yml:
- Around line 54-55: The current conditional lines that append to $GITHUB_OUTPUT
use "|| true", which masks real write failures; replace each one with an
explicit if-statement that tests the platform (the existing [[ "$platform" ==
"ios" || "$platform" == "both" ]] and [[ "$platform" == "android" || "$platform"
== "both" ]]) and only performs the echo/redirect inside the if-body so any
failure writing to $GITHUB_OUTPUT surfaces as an error; update both the ios and
android condition lines accordingly (remove "|| true" and wrap the echo >>
$GITHUB_OUTPUT in an if ...; then ...; fi block).
🧹 Nitpick comments (2)
.github/workflows/distribute_internal.yml (1)
7-8: Temporary branch trigger looks fine; consider a stronger cleanup hook.
Since this is explicitly temporary, consider linking the TODO to a tracking issue or a removal date so it doesn’t linger..github/workflows/distribute_external.yml (1)
51-52: Useifblocks instead of|| trueto avoid masking failures in workflow outputs.With
set -eenabled (GitHub Actions default for bash steps), the||operator causes the left-hand command to be considered "tested," bypassing error detection. If theechoor file redirect fails,|| truemasks the failure by returning 0 overall, making the step succeed when it should fail.♻️ Safer conditional output (preserves error visibility)
- [[ "$platform" == "ios" || "$platform" == "both" ]] && echo "run_ios=true" >> $GITHUB_OUTPUT || true - [[ "$platform" == "android" || "$platform" == "both" ]] && echo "run_android=true" >> $GITHUB_OUTPUT || true + if [[ "$platform" == "ios" || "$platform" == "both" ]]; then + echo "run_ios=true" >> "$GITHUB_OUTPUT" + fi + if [[ "$platform" == "android" || "$platform" == "both" ]]; then + echo "run_android=true" >> "$GITHUB_OUTPUT" + fi
| [[ "$platform" == "ios" || "$platform" == "both" ]] && echo "run_ios=true" >> $GITHUB_OUTPUT || true | ||
| [[ "$platform" == "android" || "$platform" == "both" ]] && echo "run_android=true" >> $GITHUB_OUTPUT || true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions bash set -e error masking with || true and && operator
💡 Result:
In GitHub Actions, a run: block on Linux/macOS is executed with bash -e by default (fail-fast / set -e). If you explicitly set shell: bash, Actions runs bash … -eo pipefail (adds pipefail). [1]
Why set -e gets “masked” by && / || (and thus by || true)
In Bash/POSIX, errexit (set -e) is ignored for commands that are part of an AND/OR list—i.e., commands on the left side of cmd1 && cmd2 or cmd1 || cmd2 (and generally any element except the final command in the list). [2][3]
So in a step that’s effectively running with set -e, these do not exit the step when dangerous fails:
dangerous || true # failure is intentionally turned into success
dangerous && echo ok # failure doesn’t exit the shell; it just skips echoThat’s the “masking”: the failure happened, but set -e won’t stop execution because Bash treats it as a conditional execution context. [2][3]
Practical fixes / safer patterns
1) If you want the step to fail, don’t hide the error with || true.
Prefer an if with an explicit exit 1:
if ! dangerous; then
echo "dangerous failed" >&2
exit 1
fi2) If you want to allow failure but still record it:
if ! dangerous; then
rc=$?
echo "dangerous failed with $rc (continuing)" >&2
fi3) If you temporarily need “no fail-fast” inside a step, disable it explicitly:
set +e
dangerous
rc=$?
set -eThis works because Actions’ default -e is just shell state within that step process. [4]
4) Pipelines: if you rely on catching failures inside pipelines, use shell: bash (to get pipefail) or set it yourself, because default is only -e. [1]
Sources
[1] GitHub Docs – default shell commands (bash -e {0}, and bash … -eo pipefail {0} when shell: bash) https://docs.github.com/en/enterprise-cloud%40latest/actions/reference/workflows-and-actions/workflow-syntax
[2] GNU bash / POSIX semantics discussed (AND-OR lists not subject to set -e except final command) https://lists.gnu.org/archive/html/bug-bash/2023-08/msg00027.html
[3] POSIX quote on -e being ignored in AND-OR lists (except last) via SO answer https://stackoverflow.com/questions/9629710/what-is-the-proper-way-to-detect-shell-exit-code-when-errexit-option-is-set
[4] GitHub Actions uses set -e in bash steps; suggestion to use set +e to continue within a step https://stackoverflow.com/questions/78922261/in-a-github-action-how-to-continue-within-a-step-even-if-a-command-returns-a-no
🌐 Web query:
bash error handling set -e with conditional && || chains vs if statements
💡 Result:
With set -e (errexit), Bash does not exit on every non-zero status. Two big exceptions are exactly the constructs you’re comparing:
1) && / || chains (AND-OR lists)
In an AND-OR list, errexit is ignored for every command except the last one in the whole chain. So a failure on the left side of && or || won’t terminate the script; it just drives the short-circuit logic. This is specified by Bash and by POSIX. [1][2]
set -e
false && echo "won't print"
echo "still running" # printsLikewise:
set -e
false || echo "prints"
echo "still running" # printsOnly if the final command in the chain fails (and you’re not in another “ignored” context) will set -e trigger an exit. [1][2]
2) if tests
The command(s) used as the if/elif condition also run with errexit ignored, so a failing test does not exit the script—it just selects the else branch. [1][2]
set -e
if false; then
echo "then"
else
echo "else" # prints
fi
echo "still running" # printsPractical guidance
- Use
if cmd; then ...when failure is expected control flow. - Use an explicit guard when failure must abort, e.g.
cmd || exit 1(orcmd || return 1in a function), instead of relying onset -einside complex&&/||logic. (This avoids the “ignored except last” rule.) [1][2]
Sources: Bash set -e (errexit) definition and exceptions [1]; POSIX definition of when -e is ignored (AND-OR lists and if/elif tests) [2].
[1] https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html
[2] https://pubs.opengroup.org/onlinepubs/9699919799.2013edition/utilities/V3_chap02.html
🌐 Web query:
GitHub Actions $GITHUB_OUTPUT echo failure handling best practices
💡 Result:
Best practices for writing to $GITHUB_OUTPUT (and not losing failures)
1) Write outputs using the environment-file format (and quote the path)
Use the official file-command pattern and append (>>) to the file; make sure the step has an id. [1]
echo "name=value" >> "$GITHUB_OUTPUT"If this write fails (disk/full, permissions, bad path), you want the step to fail—so don’t hide the exit code.
2) Make your shell fail fast so a bad write stops the step
In Bash, explicitly enable strict mode so any failing command (including the >> "$GITHUB_OUTPUT" write) fails the step/job:
set -euo pipefail3) For multiline values, don’t echo "k=$value" — use the delimiter syntax
GitHub explicitly supports multiline outputs using the same delimiter (“heredoc-style”) technique as multiline env vars. [1]
{
echo "MY_OUTPUT<<EOF"
printf '%s\n' "$value"
echo "EOF"
} >> "$GITHUB_OUTPUT"(Use a delimiter that won’t appear in the value; many people generate a GUID/UUID-like token.)
4) Prefer printf over echo for correctness
echo can be shell-dependent (e.g., -n, \ escapes). printf is more predictable—especially when output values may contain backslashes or leading dashes.
5) Use UTF‑8 when writing to runner “environment files” (Windows/PowerShell gotcha)
GitHub notes these runner files should be written using UTF‑8 to avoid processing issues. [2]
Best practices for “failure handling” around output-writing steps
6) Don’t combine continue-on-error with if: failure() expecting it to run
If a step has continue-on-error: true, a following step guarded by if: failure() may not run the way you expect (because the failure is effectively “captured”). This is a known footgun. [5]
Instead: key off the prior step’s outcome/conclusion in your if, e.g.:
- id: build
continue-on-error: true
run: ./build.sh
- name: react to build failure
if: ${{ steps.build.outcome == 'failure' }}
run: echo "build failed"(Using steps.<id>.conclusion is also common; see examples/discussion. [4])
7) For cleanup/log upload steps, prefer if: ${{ !cancelled() }} over always()
GitHub warns that always() can cause hangs/timeouts for tasks that might critically fail, and recommends if: ${{ !cancelled() }} as the alternative for “run on success or failure”. [3]
Key references
- How to set outputs via
$GITHUB_OUTPUT+ multiline technique: GitHub Docs. [1] - UTF‑8 guidance for environment files: GitHub Docs. [2]
- Status-check functions (
failure(),always(), recommended!cancelled()): GitHub Docs. [3] continue-on-error+ checkingsteps.<id>.conclusion/outcome: discussion/examples. [4]continue-on-error+failure()surprising behavior: actions/toolkit issue. [5]
Sources: [1] (docs.github.com) [2] (docs.github.com) [3] (docs.github.com) [4] (kenmuse.com) [5] (github.com)
Citations:
- 1: https://docs.github.com/actions/reference/workflow-commands-for-github-actions
- 2: https://docs.github.com/actions/reference/workflow-commands-for-github-actions
- 3: https://docs.github.com/en/enterprise-cloud%40latest/actions/reference/workflows-and-actions/expressions?utm_source=openai
- 4: https://www.kenmuse.com/blog/how-to-handle-step-and-job-errors-in-github-actions/?utm_source=openai
- 5: Wrong behaviour when combining 'continue-on-error' and 'failure()' in subsequent steps actions/toolkit#1034
Avoid masking real failures from $GITHUB_OUTPUT writes.
|| true hides errors from both the condition check and the echo/redirect itself. With set -e in GitHub Actions, replacing the || true pattern with an if block keeps the step green on false platform conditions while still failing on real write errors to $GITHUB_OUTPUT.
♻️ Safer conditional output (avoids masking errors)
- [[ "$platform" == "ios" || "$platform" == "both" ]] && echo "run_ios=true" >> $GITHUB_OUTPUT || true
- [[ "$platform" == "android" || "$platform" == "both" ]] && echo "run_android=true" >> $GITHUB_OUTPUT || true
+ if [[ "$platform" == "ios" || "$platform" == "both" ]]; then
+ echo "run_ios=true" >> "$GITHUB_OUTPUT"
+ fi
+ if [[ "$platform" == "android" || "$platform" == "both" ]]; then
+ echo "run_android=true" >> "$GITHUB_OUTPUT"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [[ "$platform" == "ios" || "$platform" == "both" ]] && echo "run_ios=true" >> $GITHUB_OUTPUT || true | |
| [[ "$platform" == "android" || "$platform" == "both" ]] && echo "run_android=true" >> $GITHUB_OUTPUT || true | |
| if [[ "$platform" == "ios" || "$platform" == "both" ]]; then | |
| echo "run_ios=true" >> "$GITHUB_OUTPUT" | |
| fi | |
| if [[ "$platform" == "android" || "$platform" == "both" ]]; then | |
| echo "run_android=true" >> "$GITHUB_OUTPUT" | |
| fi |
🤖 Prompt for AI Agents
In @.github/workflows/distribute_internal.yml around lines 54 - 55, The current
conditional lines that append to $GITHUB_OUTPUT use "|| true", which masks real
write failures; replace each one with an explicit if-statement that tests the
platform (the existing [[ "$platform" == "ios" || "$platform" == "both" ]] and
[[ "$platform" == "android" || "$platform" == "both" ]]) and only performs the
echo/redirect inside the if-body so any failure writing to $GITHUB_OUTPUT
surfaces as an error; update both the ios and android condition lines
accordingly (remove "|| true" and wrap the echo >> $GITHUB_OUTPUT in an if ...;
then ...; fi block).
2 participants
Summary by CodeRabbit