bee dl: Uncaught ArgumentCountError function bee_error_handler() when there are typos in a module name

[indigoxela]

Ran into this by accident, but it might be worth fixing:

When trying to download a module, but having a typo in the module name, the error handler chokes.

An example:

# bee dl foobartypo

There is no release for 'backdrop-contrib/foobartypo'. Do you you want to download the dev version instead? (y/N): y

# y and enter

PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function bee_error_handler(), 4 passed in .../commands/download.bee.inc on line 182 and exactly 5 expected in .../bee.php:44
Stack trace:

#0 ...commands/download.bee.inc(182): bee_error_handler()
#1 ...commands/download.bee.inc(61): download_bee_git_info()
#2 ...includes/command.inc(88): download_bee_callback()
#3 ...bee.php(23): bee_process_command()
#4 {main}
  thrown in ...bee.php on line 44

Also with the latest dev (I'm currently on b29e71d5099661e1076cee8290274edfd4d6214e).

Typos can happen easily, making dl a little more robust (or maybe the error handler?) might make sense.

[yorkshire-pudding]

I've encountered this a few times and thought it could be better. It should check to see if it can actually find a project first and then gracefully exit with an informative message that the project doesn't exist.

[herbdool]

Might be because of: https://www.php.net/manual/en/function.set-error-handler.php:

Warning This parameter has been DEPRECATED as of PHP 7.2.0, and REMOVED as of PHP 8.0.0. If your function defines this parameter without a default, an error of "too few arguments" will be raised when it is called.

I got the fatal error in PHP 8.1 (when testing with bee cron).

This fixes it:

function bee_error_handler($errno, $message, $filename = NULL, $line = NULL, $context = NULL) {
  if (error_reporting() > 0) {
    echo "$message\n";
    echo " $filename:$line\n\n";
  }
}

[ghost]

Thanks @herbdool!

@indigoxela, @yorkshire-pudding: Please see if https://github.com/backdrop-contrib/bee/pull/198 fixes this for you.

[indigoxela]

Please see if https://github.com/backdrop-contrib/bee/pull/198 fixes this for you.

Not yet, but the error changes.

Before:

PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function bee_error_handler(), 4 passed in /usr/local/bin/bee_git/commands/download.bee.inc on line 182 and exactly 5 expected in /usr/local/bin/bee_git/bee.php:44
Stack trace:
#0 /usr/local/bin/bee_git/commands/download.bee.inc(182): bee_error_handler()
#1 /usr/local/bin/bee_git/commands/download.bee.inc(61): download_bee_git_info()
#2 /usr/local/bin/bee_git/includes/command.inc(88): download_bee_callback()
#3 /usr/local/bin/bee_git/bee.php(23): bee_process_command()
#4 {main}
  thrown in /usr/local/bin/bee_git/bee.php on line 44

After:

Undefined array key "default_branch"
 /usr/local/bin/bee_git/commands/download.bee.inc:182

PHP Fatal error:  Uncaught TypeError: Cannot access offset of type string on string in /usr/local/bin/bee_git/commands/download.bee.inc:198
Stack trace:
#0 /usr/local/bin/bee_git/commands/download.bee.inc(61): download_bee_git_info()
#1 /usr/local/bin/bee_git/includes/command.inc(88): download_bee_callback()
#2 /usr/local/bin/bee_git/bee.php(23): bee_process_command()
#3 {main}
  thrown in /usr/local/bin/bee_git/commands/download.bee.inc on line 198

Tested on PHP 8.1.8.

Patch applied on latest dev (6626cd55).

[herbdool]

Looks like the patch works. It's supposed to handle other errors.

[indigoxela]

Looks like the patch works. It's supposed to handle other errors.

@herbdool With the patch applied, another fatal error is thrown, when there's for example a tiny typo in the module name.

What I'd personally expect is a text like "no such module". And I belief, the download command should be able to handle that.

To me it seems like function bee_error_handler (PHP compatibility) is fixed, but not the actual problem.

[herbdool]

I don't know how @BWPanda wants to handle it but if it's one issue pretty bug then I just mean could create another issue for other bugs since the bee_error_handler one is fixed with this.

[herbdool]

Especially when this bug affects all commands

[ElusiveMind]

I have committed a fix for this in pull request #200

[yorkshire-pudding]

Testing this in the Bee lando testing recipe with PHP 7.4 With PR #198 I get the same errors as I got before:

Error before and after #198

```bash Undefined index: default_branch /app/commands/download.bee.inc:182 Illegal string offset 'type' /app/commands/download.bee.inc:198 Illegal string offset 'type' /app/commands/download.bee.inc:198 [✘] The 'type' of project 'foobartypo' could not be determined. ```

Testing with @ElusiveMind 's PR #200 - I get a friendlier error message albeit still misleading.

Error after applying #200

```bash There is no release for 'backdrop-contrib/foobartypo'. Do you you want to download the dev version instead? (y/N): y [✘] The 'type' of project 'foobartypo' could not be determined. ```

I'm minded to agree with @indigoxela that the issue about handling module typos needs to be addressed so we can give a meaningful and accurate error message and probably stop trying to get type and branch where it doesn't exist.

I have an idea how to do that, but need to look at this further.

[yorkshire-pudding]

Thank you to everyone for input here. @ElusiveMind - Thank you for your PR - while this takes away some of the unfriendly system message it adds formatting to error messages which if we're doing needs some more thought.

I've taken the PR by @BWPanda that fixes the PHP 8 compatibility but gone further. As @indigoxela has rightly pointed out, the download function should check for a typo rather than just erroring because it can't find the type. That's what I've tried to do in #209.

@herbdool @BWPanda @indigoxela @ElusiveMind - please test and let me know any feedback.

[indigoxela]

... please test and let me know any feedback.

@yorkshire-pudding great job, many thanks for your work!

To test I tried three scenarios (with php 8.1).

1. A typo in module name:

# bee dl image_block

 [✘] The 'image_block' project repository could not be found. Please check your spelling and try again.

2. Correct spelling:

# bee dl imageblock
/tmp/imageblock633bcc2822882/image 100%[================================================================>]  19.18K  --.-KB/s    in 0.001s  

 [✔] 'imageblock' was downloaded into '/var/www/crashburn/html/modules/contrib/imageblock'.

3. A project without stable release:

# bee dl shib_auth
There is no release for 'backdrop-contrib/shib_auth'. Do you you want to download the dev version instead? (y/N): y
/tmp/shib_auth633bccd0e0468/shib_a     [ <=>                                                             ]  34.56K  --.-KB/s    in 0.008s  

 [✔] 'shib_auth' was downloaded into '/var/www/crashburn/html/modules/contrib/shib_auth'.

All correct, works for me. :+1: A tiny nit-pick re the response code evaluation (see my question in my PR comment).