Skip to content

Commit 32a3f7f

Browse files
committed
ci(railway): address PR review (idempotency, merge intent, robustness)
- railway_call: classify 'up' and 'redeploy' as non-idempotent so an ambiguous transient timeout is NOT blind-retried (avoids triggering a duplicate/racing deployment). Rate-limit retries still apply. (Two reviewers flagged this.) - railway_call + _railway_graphql: floor RAILWAY_RETRY_MAX at 1 so a '0' override can't silently return success without making the call. - upsert_service_vars: set 'replace: false' explicitly on variableCollection Upsert. Verified the API already defaults to merge (set_vars then set_optional_vars on the same service rely on accumulation), but pinning it guards against a future default change wiping earlier variables. Confirmed the field is accepted by the schema. - upsert_service_vars: if a service name can't be resolved to an id, fall back to the CLI variable set instead of hard-failing the deploy. All verified against a live preview project (references still resolve, '=' in values preserved) and unit/regression suites still pass.
1 parent 71c9492 commit 32a3f7f

2 files changed

Lines changed: 20 additions & 7 deletions

File tree

hosting/railway/oss/scripts/configure.sh

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,18 @@ upsert_service_vars() {
110110
local svc_id
111111
svc_id="$(_service_id "$service")"
112112
if [ -z "$svc_id" ]; then
113-
printf "Could not resolve service id for '%s' in environment '%s'\n" "$service" "$ENV_NAME" >&2
114-
return 1
113+
# Name didn't match the cached status JSON (e.g. unexpected casing). Don't
114+
# hard-fail the deploy where the CLI's --service would have worked; fall
115+
# back to it.
116+
printf "Could not resolve service id for '%s'; falling back to CLI variable set.\n" "$service" >&2
117+
railway_call variable set --service "$service" --environment "$ENV_NAME" --skip-deploys "$@" >/dev/null
118+
return 0
115119
fi
116120

121+
# replace:false makes the merge intent explicit: configure.sh calls set_vars
122+
# then set_optional_vars for the same service and relies on accumulation.
123+
# (Verified the API already defaults to merge, but pinning it avoids any
124+
# future default change silently wiping earlier variables.)
117125
local vars_json payload
118126
vars_json="$(_vars_to_json "$@")"
119127
payload="$(jq -nc \
@@ -122,7 +130,7 @@ upsert_service_vars() {
122130
--arg s "$svc_id" \
123131
--argjson vars "$vars_json" \
124132
'{query: "mutation($input: VariableCollectionUpsertInput!){ variableCollectionUpsert(input: $input) }",
125-
variables: {input: {projectId: $p, environmentId: $e, serviceId: $s, skipDeploys: true, variables: $vars}}}')"
133+
variables: {input: {projectId: $p, environmentId: $e, serviceId: $s, skipDeploys: true, replace: false, variables: $vars}}}')"
126134

127135
_railway_graphql "$payload" >/dev/null
128136
}

hosting/railway/oss/scripts/lib.sh

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,17 +134,21 @@ _railway_redact() {
134134
# RAILWAY_RETRY_DELAY Initial backoff delay in seconds (default: 10)
135135
railway_call() {
136136
local max_attempts="${RAILWAY_RETRY_MAX:-5}"
137+
[ "$max_attempts" -ge 1 ] 2>/dev/null || max_attempts=1
137138
local delay="${RAILWAY_RETRY_DELAY:-10}"
138139
local attempt=1
139140
local output
140141
local exit_code
141142

142-
# Is this a non-idempotent create? If so, an ambiguous timeout must not be
143-
# blind-retried (the resource may already exist). Rate-limit retries are
144-
# still safe because a 429 is rejected before any work is done.
143+
# Is this a non-idempotent / side-effecting command? If so, an ambiguous
144+
# timeout must not be blind-retried: the server may have already accepted
145+
# the request, so a retry could duplicate the resource (`init`/`add`/
146+
# `environment new`/`volume add`) or trigger a second deployment (`up`/
147+
# `redeploy`). Rate-limit retries stay safe for all commands because a 429
148+
# is rejected before any work is done.
145149
local idempotent=1
146150
case "$1" in
147-
init | add) idempotent=0 ;;
151+
init | add | up | redeploy) idempotent=0 ;;
148152
environment) [ "${2:-}" = "new" ] && idempotent=0 ;;
149153
volume) [ "${2:-}" = "add" ] && idempotent=0 ;;
150154
esac
@@ -217,6 +221,7 @@ _railway_graphql() {
217221
local endpoint="${RAILWAY_GRAPHQL_URL:-https://backboard.railway.com/graphql/v2}"
218222
local token="${RAILWAY_API_TOKEN:-${RAILWAY_TOKEN:-}}"
219223
local max_attempts="${RAILWAY_RETRY_MAX:-5}"
224+
[ "$max_attempts" -ge 1 ] 2>/dev/null || max_attempts=1
220225
local delay="${RAILWAY_RETRY_DELAY:-10}"
221226
local timeout_s="${RAILWAY_GRAPHQL_TIMEOUT:-90}"
222227
local attempt=1

0 commit comments

Comments
 (0)