Skip to content

Conversation

@NickLaMuro
Copy link
Contributor

@NickLaMuro NickLaMuro commented Jul 13, 2020

When (mistakenly) starting foreman with a empty Procfile, it ends up not being possible to exit this process via a TERM signal due to the following error:

$ foreman start
ERROR: Procfile does not exist.
$ touch Procfile
$ bin/foreman start
^Ccomparison of NilClass with 6 failed
./lib/foreman/engine/cli.rb:80:in `name_padding'
./lib/foreman/engine/cli.rb:85:in `pad_process_name'
./lib/foreman/engine/cli.rb:61:in `block in output'
./lib/foreman/engine/cli.rb:57:in `each'
./lib/foreman/engine/cli.rb:57:in `output'
./lib/foreman/engine.rb:335:in `block in output_with_mutex'
./lib/foreman/engine.rb:334:in `synchronize'
./lib/foreman/engine.rb:334:in `output_with_mutex'
./lib/foreman/engine.rb:340:in `system'
./lib/foreman/engine.rb:124:in `handle_interrupt'
./lib/foreman/engine.rb:104:in `handle_signal'
./lib/foreman/engine.rb:389:in `handle_signals'
./lib/foreman/engine.rb:412:in `block (2 levels) in watch_for_output'
./lib/foreman/engine.rb:409:in `loop'
./lib/foreman/engine.rb:409:in `block in watch_for_output'
^C^C^C^C^C^X^Z
[1]+  Stopped                 bin/foreman start
$ kill %1
$ jobs
[1]+  Running                 bin/foreman start &
$ jobs
[1]+  Running                 bin/foreman start &
$ kill -9 %1
[1]+  Killed: 9               bin/foreman start
$ jobs

By adding a .to_i, this simply allows the Engine::Cli#name_padding method to default to 6, allowing the rest of the shutdown process to finish executing and exit the process gracefully.

Alternatively, this could be fixed by not allowing an empty Procfile to be valid (was actually not addressed in #620 ), which would most likely prevent this state from being feasible, but I think having this catch as well doesn't hurt anything. I will look into creating a PR to also solve for this as well.

When (mistakenly) starting foreman with a empty Procfile, it ends up not
being possible to exit this process via a TERM signal due to the
following error:

    $ foreman start
    ERROR: Procfile does not exist.
    $ touch Procfile
    $ bin/foreman start
    ^Ccomparison of NilClass with 6 failed
    ./lib/foreman/engine/cli.rb:80:in `name_padding'
    ./lib/foreman/engine/cli.rb:85:in `pad_process_name'
    ./lib/foreman/engine/cli.rb:61:in `block in output'
    ./lib/foreman/engine/cli.rb:57:in `each'
    ./lib/foreman/engine/cli.rb:57:in `output'
    ./lib/foreman/engine.rb:335:in `block in output_with_mutex'
    ./lib/foreman/engine.rb:334:in `synchronize'
    ./lib/foreman/engine.rb:334:in `output_with_mutex'
    ./lib/foreman/engine.rb:340:in `system'
    ./lib/foreman/engine.rb:124:in `handle_interrupt'
    ./lib/foreman/engine.rb:104:in `handle_signal'
    ./lib/foreman/engine.rb:389:in `handle_signals'
    ./lib/foreman/engine.rb:412:in `block (2 levels) in watch_for_output'
    ./lib/foreman/engine.rb:409:in `loop'
    ./lib/foreman/engine.rb:409:in `block in watch_for_output'
    ^C^C^C^C^C^X^Z
    [1]+  Stopped                 bin/foreman start
    $ kill %1
    $ jobs
    [1]+  Running                 bin/foreman start &
    $ jobs
    [1]+  Running                 bin/foreman start &
    $ kill -9 %1
    [1]+  Killed: 9               bin/foreman start
    $ jobs

By adding a `.to_i`, this simply allows the `Engine::Cli#name_padding`
method to default to 6, allowing the rest of the shutdown process to
finish executing and exit the process gracefully.
@ddollar
Copy link
Owner

ddollar commented Apr 12, 2024

Looks like a good check, should also be covered by #770

Thank you!

@ddollar ddollar merged commit fb80608 into ddollar:main Apr 12, 2024
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