Skip to content

feat(x2a): Error handling - error from python log #2585

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

Open
elai-shalev wants to merge 4 commits into
base: main
Choose a base branch
from
+48 −24
Open

feat(x2a): Error handling - error from python log #2585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a2f6a79
Error handling - error from python log
elai-shalev Mar 22, 2026
eeffac3
Simplify reporting by reading from file
elai-shalev Mar 23, 2026
d5c0b2d
applied review
elai-shalev Mar 30, 2026
65e349f
changeset
elai-shalev Mar 30, 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
5 changes: 5 additions & 0 deletions workspaces/x2a/.changeset/error-propagation-fixes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@red-hat-developer-hub/backstage-plugin-x2a-backend': patch
---

Improve error propagation in job script: consolidate error handling into run_x2a function with default message and appended error details, add command logging, and refactor publish-aap to use the shared error handler.
67 changes: 43 additions & 24 deletions workspaces/x2a/plugins/x2a-backend/templates/x2a-job-script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
TERMINATED=false
COMMIT_ID=""

# Path where x2a-convertor writes error details on failure
export X2A_ERROR_FILE="/tmp/x2a-error.txt"

# Report job result back to the backend.
report_result() {
local status="$1"
Expand Down Expand Up @@ -74,6 +77,38 @@
fi
}

# Run an x2a tool command with error reporting.
# On failure, reads the error details file written by x2a-convertor and sets ERROR_MESSAGE.
# On success, clears ERROR_MESSAGE.
# Captured output is stored in X2A_OUTPUT for callers that need to parse it.
# Usage: run_x2a uv run app.py <phase> [args...]
run_x2a() {

Check warning on line 85 in workspaces/x2a/plugins/x2a-backend/templates/x2a-job-script.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add an explicit return statement at the end of the function. 


See more on https://sonarcloud.io/project/issues?id=redhat-developer_rhdh-plugins&issues=AZ0amOgWNWh-T1cKrMU5&open=AZ0amOgWNWh-T1cKrMU5&pullRequest=2585
Copy link
Copy Markdown
Contributor

@eloycoto eloycoto Mar 23, 2026

Choose a reason for hiding this comment

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

now that we have this fucntion, what about if we add the log here:

echo "Command: uv run app.py init --source-dir ${SOURCE_BASE} \"${USER_REQ}\""

because it's already in all phases, and can be log here without issues, no?

rm -f "${X2A_ERROR_FILE}"

echo "Command: $*"

local tmpfile
tmpfile=$(mktemp)

set +e
"$@" 2>&1 | tee "${tmpfile}"
local rc=${PIPESTATUS[0]}
set -e

X2A_OUTPUT=$(cat "${tmpfile}")
rm -f "${tmpfile}"

if [ ${rc} -ne 0 ]; then

Check failure on line 101 in workspaces/x2a/plugins/x2a-backend/templates/x2a-job-script.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=redhat-developer_rhdh-plugins&issues=AZ0amOgWNWh-T1cKrMU6&open=AZ0amOgWNWh-T1cKrMU6&pullRequest=2585
ERROR_MESSAGE="Unexpected error during ${PHASE} phase. See the job log for details."
if [ -f "${X2A_ERROR_FILE}" ]; then

Check failure on line 103 in workspaces/x2a/plugins/x2a-backend/templates/x2a-job-script.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich. 


See more on https://sonarcloud.io/project/issues?id=redhat-developer_rhdh-plugins&issues=AZ0amOgWNWh-T1cKrMU7&open=AZ0amOgWNWh-T1cKrMU7&pullRequest=2585
ERROR_MESSAGE+=" Message: $(cat "${X2A_ERROR_FILE}")"
fi
Comment on lines +103 to +105
Copy link
Copy Markdown

@rhdh-qodo-merge rhdh-qodo-merge bot Mar 23, 2026

Choose a reason for hiding this comment

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

Action required

1. run_x2a fallback lacks exit code 📎 Requirement gap ✧ Quality

When x2a-convertor fails and ${X2A_ERROR_FILE} is missing, the new fallback sets ERROR_MESSAGE
to a generic string instead of the required Script failed with exit code N. This can prevent the
backend from surfacing the mandated fallback errorDetails format.
Agent Prompt 
## Issue description
`run_x2a()` sets a generic `ERROR_MESSAGE` when `${X2A_ERROR_FILE}` is missing, but compliance requires the fallback to be `Script failed with exit code N`.

## Issue Context
The backend relies on `ERROR_MESSAGE` for `errorDetails`. If no meaningful convertor error can be extracted, the required fallback format includes the exit code.

## Fix Focus Areas
- workspaces/x2a/plugins/x2a-backend/templates/x2a-job-script.sh[93-97]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

exit ${rc}
fi

ERROR_MESSAGE=""
}

# Cleanup trap: fires on every exit (success or failure).
# Guarantees exactly one report_result call regardless of how the script ends.
cleanup() {
Expand Down Expand Up @@ -226,10 +261,7 @@
# Usage: app.py init [OPTIONS] USER_REQUIREMENTS
# --source-dir DIRECTORY Source directory to analyze
USER_REQ="${USER_PROMPT:-Analyze the Chef cookbooks and create a migration plan}"
echo "Command: uv run app.py init --source-dir ${SOURCE_BASE} \"${USER_REQ}\""
ERROR_MESSAGE="Unexpected error during init phase. See the job log for details."
uv run app.py init --source-dir "${SOURCE_BASE}" "${USER_REQ}"
ERROR_MESSAGE=""
run_x2a uv run app.py init --source-dir "${SOURCE_BASE}" "${USER_REQ}"

# Copy output to target location
# Note: x2a tool writes files to the source directory (--source-dir)
Expand Down Expand Up @@ -290,10 +322,7 @@
echo "Working directory: $(pwd)"

USER_REQ="${USER_PROMPT:-Analyze the module '${MODULE_NAME}' for migration to Ansible}"
echo "Command: uv run app.py analyze --source-dir ${SOURCE_BASE} \"${USER_REQ}\""
ERROR_MESSAGE="Unexpected error during analyze phase. See the job log for details."
uv run app.py analyze --source-dir "${SOURCE_BASE}" "${USER_REQ}"
ERROR_MESSAGE=""
run_x2a uv run app.py analyze --source-dir "${SOURCE_BASE}" "${USER_REQ}"

# Copy output to target location
# Note: x2a tool produces migration-plan-{module_name}.md (spaces replaced with underscores)
Expand Down Expand Up @@ -347,15 +376,12 @@
echo "Working directory: $(pwd)"

USER_REQ="${USER_PROMPT:-Migrate this module to Ansible}"
echo "Command: uv run app.py migrate --source-dir ${SOURCE_BASE} --source-technology Chef --high-level-migration-plan ${PROJECT_PATH}/migration-plan.md --module-migration-plan ${OUTPUT_DIR}/migration-plan-${MODULE_NAME_SANITIZED}.md \"${USER_REQ}\""
ERROR_MESSAGE="Unexpected error during migrate phase. See the job log for details."
uv run app.py migrate \
run_x2a uv run app.py migrate \
--source-dir "${SOURCE_BASE}" \
--source-technology Chef \
--high-level-migration-plan "${PROJECT_PATH}/migration-plan.md" \
--module-migration-plan "${OUTPUT_DIR}/migration-plan-${MODULE_NAME_SANITIZED}.md" \
"${USER_REQ}"
ERROR_MESSAGE=""

# Copy output to target location
# Note: x2a tool writes to ansible/roles/{module}/ in the source directory
Expand Down Expand Up @@ -397,15 +423,11 @@

# Step 1: publish-project — assemble Ansible project from migrated role
echo "=== Step 1: Assembling Ansible project ==="
echo "Command: uv run app.py publish-project ${PROJECT_DIR} ${MODULE_NAME}"

# publish-project reads from {project_id}/modules/{module_name}/ansible/roles/{module_name}/
# and writes to {project_id}/ansible-project/
# It operates relative to CWD, so we run from TARGET_BASE
pushd "${TARGET_BASE}"
ERROR_MESSAGE="Unexpected error during publish phase (publish-project). See the job log for details."
uv run --project /app /app/app.py publish-project "${PROJECT_DIR}" "${MODULE_NAME}"
ERROR_MESSAGE=""
run_x2a uv run --project /app /app/app.py publish-project "${PROJECT_DIR}" "${MODULE_NAME}"
popd

# Verify ansible-project was created
Expand All @@ -422,17 +444,14 @@
# Step 2: publish-aap — register with AAP and sync
echo ""
echo "=== Step 2: Publishing to AAP ==="
echo "Command: uv run app.py publish-aap --target-repo ${TARGET_REPO_URL} --target-branch ${TARGET_REPO_BRANCH} --project-id ${PROJECT_DIR}"
cd /app
ERROR_MESSAGE="Unexpected error during publish phase (publish-aap). See the job log for details."
PUBLISH_OUTPUT=$(uv run app.py publish-aap \
run_x2a uv run app.py publish-aap \
--target-repo "${TARGET_REPO_URL}" \
--target-branch "${TARGET_REPO_BRANCH}" \
--project-id "${PROJECT_DIR}" 2>&1 | tee /dev/stderr)
ERROR_MESSAGE=""
--project-id "${PROJECT_DIR}"

# Parse AAP project ID from output and construct URL
AAP_PROJECT_ID=$(echo "${PUBLISH_OUTPUT}" | grep -oP 'ID: \K[0-9]+' | tail -1)
# Parse AAP project ID from captured output
AAP_PROJECT_ID=$(echo "${X2A_OUTPUT}" | grep -oP 'ID: \K[0-9]+' | tail -1)
if [ -n "${AAP_PROJECT_ID}" ]; then
ARTIFACTS+=("ansible_project:${AAP_CONTROLLER_URL}/execution/projects/${AAP_PROJECT_ID}/details")
else
Expand Down
Loading