Skip to content
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.

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

deploy:check failing does not trigger deploy:failed or deploy:finished #1993

Open
vpillinger opened this issue Jun 26, 2018 · 4 comments
Open
Labels

Comments

@vpillinger
Copy link

vpillinger commented Jun 26, 2018

Steps to reproduce

Add this to a default capistrano project. Do not create .env on the server.

set :linked_files, fetch(:linked_files, []).push('.env')

namespace :deploy do
  desc 'Ending Hook'
  task :ending_hook do
      on roles(:all) do |host|
        puts'ending'
      end
  end
end
before 'deploy:finished', 'deploy:ending_hook '
after 'deploy:failed', 'deploy:ending_hook '

Edit: Fixed rending_hook typo
Then run deploy command.

Expected behavior

The deploy will fail at deploy:check for .env file, then the ending hook will be fired for either deploy:failed or deploy:finished.

Actual behavior

The build stops but no ending hooks are fired.

System configuration

Environment

Ruby     ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux-gnu]
Rubygems 2.7.6
Bundler  1.16.2
Command  /usr/local/bin/cap staging doctor

Gems

capistrano            3.6.0  (update available)    capistrano            3.6.0  (update available)
airbrussh             1.3.0
rake                  12.3.1
sshkit                1.16.1
net-ssh               5.0.2
capistrano-composer   0.0.6
capistrano-env-config 0.3.0
capistrano-harrow     0.5.3
capistrano-ssh-doctor 1.0.0
capistrano-wpcli      0.1.4
@will-in-wi
Copy link
Contributor

Is it intentional that the before and after definitions involve rending_hook and the task's name is ending_hook?

You can also run capistrano with --trace to see the exact stuff that was run.

@vpillinger
Copy link
Author

vpillinger commented Jun 26, 2018

No, "rending_hook" was bad copy and paste. I changed the original hook names for confidentially and the original name started with an "r"

The failure hook was working when the deploy failed because of an interrupt, like ctrl+C, but not if I let it get to where it would fail via deploy:check

@mattbrictson
Copy link
Member

Interesting! Looking at the code, that is indeed the behavior. That is, when a deploy fails due to a linked file not existing, the deploy:failed task is not invoked.

The exit 1 means that Capistrano exits immediately without any opportunity for failure tasks to be run.

unless test "[ -f #{file} ]"
error t(:linked_file_does_not_exist, file: file, host: host)
exit 1
end

This behavior has been in place ever since Capistrano 3.0 was released.

I don't know if this behavior was intentional when it was written 5 years ago, or if this is a bug. Even if it is a bug (and I am inclined to agree it is) I am hesitant to change this behavior since it has been this way for 5 years and this is the first time the problem has ever been reported, to my knowledge.

The workaround would be for you to redefine the deploy:linked_files task to replace the exit 1 line with something like raise StandardError instead.

@will-in-wi and @leehambley is this something we should fix? I guess the risk is that there users are relying on the current behavior, because it would be a breaking change for them.

@will-in-wi
Copy link
Contributor

I'd want to do some investigating in regards to what happens if we remove the exit (or move it elsewhere). I think in this case we do want it to exit with a status code of 1, but it'd be ideal if we could finish running the tasks. In short, I think this could be fixed/improved. I'll take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants