Skip to content

Add fail-on-error to Scripts#960

Merged
saeedvaziry merged 2 commits into
vitodeploy:4.xfrom
emredipi:3.x
Dec 18, 2025
Merged

Add fail-on-error to Scripts#960
saeedvaziry merged 2 commits into
vitodeploy:4.xfrom
emredipi:3.x

Conversation

@emredipi
Copy link
Copy Markdown
Contributor

Problem

Scripts executed in runScript() would continue running even when a command failed. For example, if a migration failed or git pull errored, the script wouldn't stop and would keep executing subsequent commands. This could lead to incorrect deployment states.

Solution

Added set -e and set -o pipefail at the beginning of scripts. Now when any command fails, the script stops and the SSH exec method properly throws an error.

  • set -e: Script stops when a command returns a non-zero exit code
  • set -o pipefail: Enables error checking in pipes as well

Added set -e and set -o pipefail to scripts executed in runScript method.
This ensures that when commands like migration or git pull fail, the script
stops execution and subsequent commands are not executed.

Previously, scripts would continue even when a command failed, which could
lead to incorrect results and inconsistent system states.
@saeedvaziry
Copy link
Copy Markdown
Member

I don't think its a good idea to merge this to V3. it will bring breaking changes to some scripts that even throwing error in them is considered fine and script has to finish executing.

We can have this on 4.x instead however on 4.x we might need this in the SSH.php for all commands

@emredipi
Copy link
Copy Markdown
Contributor Author

Hi @saeedvaziry,

I understand the breaking change concern, and you’re right about that.

That said, the current behavior has a real downside: when a command fails, we don’t get any signal or warning at all. From my perspective, it’s hard to imagine someone intentionally relying on completely silent failures. Even if a script finishes successfully, it may leave the system in an incorrect or partially deployed state without anyone noticing.

So I think there is a clear problem here. This particular solution might not be the final answer for V3, but the underlying issue is definitely something we should address in one way or another.

@emredipi
Copy link
Copy Markdown
Contributor Author

Also worth noting: even if something goes wrong during a deployment, the process still returns a success response, which makes the failure completely invisible from the outside.
Ekran Resmi 2025-12-16 13 58 13

@saeedvaziry
Copy link
Copy Markdown
Member

My concern is not other user's scripts will break. everyone who uses deployment scripts and the main scripts they can already put set -e on top of the script and it will fail if error happens.

My main concern is Vito itself and all the places Vito relies on script getting finished no matter if it fails. I cannot test every feature of vito on a real server with different conditions to make sure this works so thats why I'd like to have such feature on the new major version that we will anyway test every part and have a beta period.

@saeedvaziry saeedvaziry changed the base branch from 3.x to 4.x December 18, 2025 19:47
@saeedvaziry saeedvaziry changed the title fix(ssh): Stop script execution on command errors Add fail-on-error to Scripts Dec 18, 2025
@saeedvaziry saeedvaziry merged commit 9ef242c into vitodeploy:4.x Dec 18, 2025
@saeedvaziry
Copy link
Copy Markdown
Member

merged to 4.x, will optimize it when get to work with the general fail-on-error topic

@emredipi
Copy link
Copy Markdown
Contributor Author

Thank you @saeedvaziry 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants