Skip to content

ci: retry Railway preview service discovery #2800

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.

Sign up for GitHub

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
amikofalvy merged 3 commits into from
Mar 26, 2026
Merged

ci: retry Railway preview service discovery #2800

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
29365d9
ci: retry Railway preview service discovery
vnv-varun Mar 23, 2026
54759a6
ci: propagate Railway discovery failures
vnv-varun Mar 23, 2026
a4ed838
Merge branch 'main' into varun/preview-railway-service-discovery
amikofalvy Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 51 additions & 11 deletions .github/scripts/preview/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,55 @@ EOF
jq -r --arg service_name "${service_name}" '.data.environment.serviceInstances.edges[] | select(.node.serviceName == $service_name) | .node.serviceId' <<< "${response}"
}

railway_wait_for_environment_id() {
local project_id="$1"
local env_name="$2"
local max_attempts="${3:-30}"
local sleep_seconds="${4:-2}"
local attempt=""
local env_id=""

for attempt in $(seq 1 "${max_attempts}"); do
env_id="$(railway_environment_id "${project_id}" "${env_name}")"
if [ -n "${env_id}" ]; then
printf '%s' "${env_id}"
return 0
fi

if [ "${attempt}" -lt "${max_attempts}" ]; then
sleep_with_jitter "${sleep_seconds}"
fi
done

echo "Unable to resolve Railway environment ID for ${env_name}." >&2
return 1
}

railway_wait_for_service_id_for_env() {
local env_id="$1"
local service_name="$2"
local env_name="$3"
local max_attempts="${4:-30}"
local sleep_seconds="${5:-2}"
local attempt=""
local service_id=""

for attempt in $(seq 1 "${max_attempts}"); do
service_id="$(railway_service_id_for_env "${env_id}" "${service_name}")"
if [ -n "${service_id}" ]; then
printf '%s' "${service_id}"
return 0
fi

if [ "${attempt}" -lt "${max_attempts}" ]; then
sleep_with_jitter "${sleep_seconds}"
fi
done

echo "Unable to resolve Railway service ID for ${service_name} in ${env_name}." >&2
return 1
}

railway_ensure_tcp_proxy() {
local project_id="$1"
local env_name="$2"
Expand All @@ -199,17 +248,8 @@ railway_ensure_tcp_proxy() {
local active=""
local attempt=""

env_id="$(railway_environment_id "${project_id}" "${env_name}")"
if [ -z "${env_id}" ]; then
echo "Unable to resolve Railway environment ID for ${env_name}." >&2
return 1
fi

service_id="$(railway_service_id_for_env "${env_id}" "${service_name}")"
if [ -z "${service_id}" ]; then
echo "Unable to resolve Railway service ID for ${service_name} in ${env_name}." >&2
return 1
fi
env_id="$(railway_wait_for_environment_id "${project_id}" "${env_name}" "${max_attempts}" "${sleep_seconds}")"
service_id="$(railway_wait_for_service_id_for_env "${env_id}" "${service_name}" "${env_name}" "${max_attempts}" "${sleep_seconds}")"
Copy link
Contributor

@pullfrog pullfrog bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): railway_ensure_tcp_proxy forwards its full max_attempts/sleep_seconds to each of the two new wait functions, then runs its own polling loop with the same budget. Worst-case wall time tripled from ~60 s to ~180 s (30 × 2 s × 3 phases) compared to the previous single-phase version.

This is probably fine in practice — the environment and service IDs should resolve quickly — but it's worth being aware of. If CI timeout pressure becomes a concern later, consider giving the discovery phases a smaller attempt budget (e.g., 10) while keeping the TCP-proxy polling at 30.

Copy link
Contributor

@claude claude bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 MAJOR: Missing error propagation from wait functions

Issue: If railway_wait_for_environment_id fails and returns exit code 1, the script continues with an empty env_id. The subsequent railway_wait_for_service_id_for_env call then retries 30 times with an empty environment ID before failing with a misleading error.

Why: Since this file is sourced and "callers own shell strictness" (per line 2), set -e cannot be assumed. The original code had explicit -z checks that prevented this cascading failure. Without explicit error handling, failures in the first lookup waste ~60 seconds retrying the second lookup with invalid input.

Fix: (1-click apply)

Suggested change
env_id="$(railway_wait_for_environment_id "${project_id}" "${env_name}" "${max_attempts}" "${sleep_seconds}")"
service_id="$(railway_wait_for_service_id_for_env "${env_id}" "${service_name}" "${env_name}" "${max_attempts}" "${sleep_seconds}")"
env_id="$(railway_wait_for_environment_id "${project_id}" "${env_name}" "${max_attempts}" "${sleep_seconds}")" || return 1
service_id="$(railway_wait_for_service_id_for_env "${env_id}" "${service_name}" "${env_name}" "${max_attempts}" "${sleep_seconds}")" || return 1

Refs:

Copy link
Contributor Author

@vnv-varun vnv-varun Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 54759a603 by restoring explicit error propagation on both wait-command substitutions with || return 1. That preserves the original fail-fast behavior when environment discovery fails.


response="$(
railway_graphql "$(cat <<EOF
Expand Down
Loading