Skip to content

Commit 13ae07e

Browse files
mikhail-dclclaude
andcommitted
fix: address PR review — curl timeouts, errexit in substitutions, statusless edge
- --max-time 30 on GitHub API and webhook curls - shopt -s inherit_errexit so failures inside $(render/jq) assignments propagate - all-statusless page keeps both placeholder lines instead of contradicting itself - new flow scenarios: statuses-endpoint failure, statusless; superseded v1 plan doc untracked Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c59f009 commit 13ae07e

5 files changed

Lines changed: 57 additions & 790 deletions

File tree

.github/actions/slack-canvas-status/test/mock_api.py

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
"""Mock GitHub API + Slack workflow webhook for update-canvas.sh flow tests.
33
44
Scenario is selected with the MOCK_SCENARIO env var:
5-
normal - dev's newest deployment succeeded; prd's newest failed, an older one succeeded.
6-
degraded - dev's newest deployment has no statuses yet, the next one failed, none
7-
succeeded; prd has no deployments.
8-
empty - no deployments in either environment.
9-
gh_error - the deployments endpoint returns HTTP 500.
10-
webhook_fail - deployment data as in normal, but the webhook responds HTTP 500.
5+
normal - dev's newest deployment succeeded; prd's newest failed, an older one succeeded.
6+
degraded - dev's newest deployment has no statuses yet, the next one failed, none
7+
succeeded; prd has no deployments.
8+
statusless - dev's only deployment has no statuses; prd has no deployments.
9+
empty - no deployments in either environment.
10+
gh_error - the deployments endpoint returns HTTP 500.
11+
gh_status_error - deployment data as in normal, but the statuses endpoint returns HTTP 500.
12+
webhook_fail - deployment data as in normal, but the webhook responds HTTP 500.
1113
Every request is appended to MOCK_LOG as one JSON line: {"path": ..., "body": ...},
1214
except GET /ping (a readiness probe) which is answered 200 and not logged.
1315
"""
@@ -64,6 +66,11 @@
6466
"prd": [],
6567
}
6668

69+
DEPLOYMENTS_STATUSLESS = {
70+
"dev": [DEPLOYMENTS_DEGRADED["dev"][0]],
71+
"prd": [],
72+
}
73+
6774
STATUSES = {
6875
2: {"state": "success", "created_at": "2026-07-18T10:00:00Z"},
6976
12: {"state": "failure", "created_at": "2026-07-19T08:30:00Z"},
@@ -100,11 +107,16 @@ def do_GET(self):
100107
self._respond([])
101108
elif SCENARIO == "degraded":
102109
self._respond(DEPLOYMENTS_DEGRADED.get(environment, []))
110+
elif SCENARIO == "statusless":
111+
self._respond(DEPLOYMENTS_STATUSLESS.get(environment, []))
103112
else:
104113
self._respond(DEPLOYMENTS.get(environment, []))
105114
return
106115
match = re.fullmatch(r"/repos/[^/]+/[^/]+/deployments/(\d+)/statuses", parsed.path)
107116
if match:
117+
if SCENARIO == "gh_status_error":
118+
self._respond({"message": "status boom"}, status=500)
119+
return
108120
status = STATUSES.get(int(match.group(1)))
109121
self._respond([status] if status else [])
110122
return

.github/actions/slack-canvas-status/test/test-flow.sh

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,25 @@ grep -q '"/webhook"' "$MOCK_LOG" || fail "expected webhook request to be recorde
9898
grep -q '500' "$ERR_FILE" || fail "expected HTTP 500 in stderr, got: $(cat "$ERR_FILE")"
9999
grep -q 'trigger_failed' "$ERR_FILE" || fail "expected webhook body in stderr, got: $(cat "$ERR_FILE")"
100100

101-
# Scenario 5: degraded — dev's newest deployment has no statuses yet (skipped for
101+
# Scenario 5: the statuses endpoint errors -> loud non-zero exit with the body,
102+
# webhook never called (pins latest_status's explicit error handler).
103+
start_server gh_status_error
104+
if bash ../update-canvas.sh 2>"$ERR_FILE"; then
105+
fail "expected non-zero exit when the statuses endpoint fails"
106+
fi
107+
stop_server
108+
grep -q 'status boom' "$ERR_FILE" || fail "expected statuses error body in stderr, got: $(cat "$ERR_FILE")"
109+
! grep -q '"/webhook"' "$MOCK_LOG" || fail "webhook must not be called when the statuses endpoint fails"
110+
111+
# Scenario 6: every deployment is statusless -> both placeholders stay "—"
112+
# (no contradictory "no recent success" next to a last-deploy of "—").
113+
start_server statusless
114+
bash ../update-canvas.sh
115+
stop_server
116+
[[ "$(payload_field dev_running)" == "" ]] || fail "statusless dev_running mismatch: $(payload_field dev_running)"
117+
[[ "$(payload_field dev_last_deploy)" == "" ]] || fail "statusless dev_last_deploy mismatch: $(payload_field dev_last_deploy)"
118+
119+
# Scenario 7: degraded — dev's newest deployment has no statuses yet (skipped for
102120
# the last-deploy line), the next one failed, none succeeded; prd is empty.
103121
start_server degraded
104122
bash ../update-canvas.sh

.github/actions/slack-canvas-status/update-canvas.sh

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
# Optional env: GITHUB_API_BASE (defaults to https://api.github.com; tests
1212
# point it at a mock).
1313
set -euo pipefail
14+
# Command substitutions must inherit errexit, or a failure inside
15+
# $(render_*)/$(jq …) assignments would be silently swallowed.
16+
shopt -s inherit_errexit
1417

1518
GITHUB_API_BASE="${GITHUB_API_BASE:-https://api.github.com}"
1619
DEPLOY_TASK="dcl/container-deployment"
@@ -71,12 +74,15 @@ render_running_line() {
7174
}
7275

7376
render_last_deploy_line() {
74-
local state="$1" ref="$2" sha="$3" time="$4"
75-
echo "$(state_emoji "$state") ${state} · $(short_ref "$ref") @ $(short_sha "$sha") · $(fmt_time "$time")"
77+
local state="$1" ref="$2" sha="$3" time="$4" ts
78+
# Assigned separately so a fmt_time failure propagates (with inherit_errexit)
79+
# instead of being swallowed inside echo's argument list.
80+
ts="$(fmt_time "$time")"
81+
echo "$(state_emoji "$state") ${state} · $(short_ref "$ref") @ $(short_sha "$sha") · ${ts}"
7682
}
7783

7884
gh_get() {
79-
curl -sS --fail-with-body \
85+
curl -sS --fail-with-body --max-time 30 \
8086
-H "Authorization: Bearer ${GITHUB_TOKEN}" \
8187
-H "Accept: application/vnd.github+json" \
8288
"${GITHUB_API_BASE}$1"
@@ -139,8 +145,12 @@ collect_env() {
139145
done
140146

141147
# Deployments exist but none of the recent ones succeeded — say so instead of
142-
# rendering the ambiguous "nothing deployed" placeholder.
143-
RUNNING_LINE="⚠️ no recent success (last ${count} deploys)"
148+
# rendering the ambiguous "nothing deployed" placeholder. Skipped when every
149+
# deployment was statusless (no last-deploy line either): both placeholders
150+
# stay "—" rather than contradicting each other.
151+
if [[ "$LAST_LINE" != "$EMPTY_LINE" ]]; then
152+
RUNNING_LINE="⚠️ no recent success (last ${count} deploys)"
153+
fi
144154
}
145155

146156
main() {
@@ -178,7 +188,7 @@ main() {
178188
}')"
179189

180190
local response
181-
response="$(printf '%s' "$payload" | curl -sS --fail-with-body \
191+
response="$(printf '%s' "$payload" | curl -sS --fail-with-body --max-time 30 \
182192
-H "Content-Type: application/json; charset=utf-8" \
183193
--data-binary @- "$SLACK_WEBHOOK_URL")" || {
184194
echo "::error::Slack webhook rejected the payload: ${response}" >&2

0 commit comments

Comments
 (0)