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

DietPi-Globals | Add new error handled command wrapper G_EXEC #3440

Merged
merged 49 commits into from
Mar 29, 2020
Merged

Conversation

MichaIng
Copy link
Owner

@MichaIng MichaIng commented Mar 25, 2020

Status: Ready

Commit list/description:

  • DietPi-Globals | Add new error handled command wrapper G_EXEC which combines G_RUN_CMD and G_ERROR_HANDLER. Output redirects can be done via "eval".

+ DietPi-Globals | Add new error handled command wrapper G_EXEC which combines G_RUN_CMD and G_ERROR_HANDLER. Output redirects can be done via "eval".
@MichaIng MichaIng added this to the v6.29 milestone Mar 25, 2020
@MichaIng MichaIng linked an issue Mar 25, 2020 that may be closed by this pull request
MichaIng added 25 commits March 26, 2020 10:09
+ DietPi-Set_swapfile | Use new G_EXEC
+ DietPi-PREP | Use new G_EXEC
+ DietPi-Explorer | Use new G_EXEC
+ DietPi-LetsEncrypt | Use new G_EXEC
+ DietPi-Drive_Manager | Use new G_EXEC
+ DietPi-NordVPN | Use new G_EXEC
+ DietPi-Update | Use new G_EXEC
+ DietPi-Services | Use new G_EXEC
+ DietPi-Login | Use new G_EXEC
+ DietPi-Patch | Use new G_EXEC
+ DietPi-Run_NTPD | Use new G_EXEC
+ DietPi-Fan_control | Use new G_EXEC
+ DietPi-Survey | Use new G_EXEC
+ DietPi-Imager | Use new G_EXEC
+ DietPi-Set_software | Use new G_EXEC
+ DietPi-Sync | Use new G_EXEC
+ DietPi-Set_hardware | Use new G_EXEC
+ DietPi-Config | Use new G_EXEC
+ DietPi-Bugreport | Upload new G_EXEC error report file
+ DietPi-PREP | Replace manual G_ERROR_HANDLER with G_EXEC eval combination as well
+ DietPi-Software | Use new G_EXEC, ToDo: Apply individual handler for Nextcloud/ownCloud occ install
+ DietPi-Globals | G_EXEC: Allow internal non-interactive retries via $G_EXEC_ATTEMPTS
+ DietPi-Config | Locales: Replace G_FILE_EXISTS with an offer to (re)install the package that provides the file
+ DietPi-Globals | G_EXEC: Add options to redirect STDOUT and run an additional function before every core function execution
+ DietPi-Globals | Remove G_FILE_EXISTS which we used only a single time and in very most cases it is required to check for file or dir only, allow symlinks or explicitly do not, or similar
+ DietPi-Globals | Use G_EXEC to handle G_CHECK_URL and all G_AG* functions but G_AGP (ToDo)
+ DietPi-Globals | Migrate G_AGP to G_EXEC as well
MichaIng added 20 commits March 27, 2020 12:55
+ DietPi-Globals | G_EXEC: Rename G_EXEC_ATTEMPTS to G_EXEC_RETRIES since a value of 1 does not mean 1 attempt but indeed 1 retry, hence two attempts
+ DietPi-Globals | G_EXEC: Remove G_EXEC_REDIRECT, since in most cases where we write to a file via echo/cat, the write redirect/write itself needs to be error handled, not the cat/echo call, which nearly never fails. This needs to more thoughts and development. Also this is basically to avoid "eval" for safety reasons, for the cases we use there is no risk. Even if we use "eval echo $VAR > file" and $VAR contains the syntax for a dangerous command substitution, only the variable gets expended but the contained command is not executed, which would require "eval eval echo $VAR > file".
+ DietPi-Globals | G_EXEC: Name G_EXEC_RETRY_FUNC to G_EXEC_PRE_FUNC and add G_EXEC_POST_FUNC e.g. to handle errors without exit code
+ DietPi-Software | Run ownCloud/Nextcloud occ/ncc via G_EXEC as well, use new G_EXEC_POST_FUNC to check for script internal errors which do not result in a PHP error exit code
+ DietP-LED_control | Use new G_EXEC
+ DietPi-Config | Use G_EXEC_NOHALT instead of G_ERROR_HANDLER_INFO_ONLY and direct function exit code instead of G_ERROR_HANDLER_EXITCODE_RETURN
+ DietPi-Globals | Remove G_ERROR_HANDLER
+ DietPi-Globals | Variables cannot be applied via G_EXEC argument: "/boot/dietpi/func/dietpi-globals: line 927: DEBIAN_FRONTEND=noninteractive: command not found"
+ DietPi-Globals | G_EXEC: Fix line background colour for GitHub error template print
+ DietPi-Patch | Use G_AGI ./package.deb instead of dpkg --force-hold,confdef,confold -i
+ DietPi-Globals | G_EXEC: Add $G_EXEC_OUTPUT_COL to allow overriding command output colour
+ DietPi-Globals | G_EXEC: Add ", please wait..." after initial info print, if G_EXEC_OUTPUT=1 is set, as it might take a while until the command produces output and we have no processing animation then
+ DietPi-Config | Use dietpi.com as default for connection test
+ DietPi-Globals | G_AGUP: Whoopsie
+ DietPi-Globals | G_AGI/G_AGP: Do not print ", please wait..." in yellow
+ DietPi-Globals | G_EXEC: Whoopsie, without "-e", echo does not expend colour codes, use printf instead which omits newline as well
+ DietPi-Software | Avoid doubled "..."
+ DietPi-Globals | G_EXEC: Do not override command exit code when $G_EXEC_OUTPUT_COL is given
+ DietPi-Globals | G_EXEC unset G_EXEC_POST_FUNC as well
+ DietPi-Software | ownCloud/Nextcloud: Omit occ/ncc output, especially to avoid confusion in case of "Following symlinks is not allowed" error on NC which we handle gracefully
+ DietPi-Globals | G_EXEC: Assure to unset functions and variables as such, to avoid failure if a same-named variable or function, respectively exist. "unset" tries to unset input as variable first and only if that fails tries to unset the function
+ DietPi-Software | ownCloud: Disable and remove Lighttpd config as well
@MichaIng
Copy link
Owner Author

MichaIng commented Mar 29, 2020

Hmm strange issue:

ownCloud install:

...
G_EXEC_DESC='ownCloud occ install'
# - Replace password string internally to avoid printing it to console
G_EXEC_PRE_FUNC(){ acommand[10]=$GLOBAL_PW acommand[14]=$GLOBAL_PW; }
# - Checking output for stack trace to handle internal errors that do not lead to php error exit code
G_EXEC_POST_FUNC(){ grep -qi 'Stack trace' $fp_log && exit_code=1337; }
G_EXEC occ maintenance:install --no-interaction --database 'mysql' --database-name 'owncloud' --database-user 'tmp_root' --database-pass '<omitted>' --admin-user "$username" --admin-pass '<omitted>' --data-dir "$datadir" | tee fp_output
...
G_EXEC systemctl restart redis-server

Output:

[  OK  ] DietPi-Software | ownCloud occ install
...
[FAILED] DietPi-Software | ownCloud occ install
 - Command: systemctl restart redis-server

The error prompt reveals that the command array contains an entry dietpi, as added by G_EXEC_PRE_FUNC above (default password), the mismatch between "[FAILED]" print and actual command name below shows that G_EXEC_DESC and G_EXEC_PRE_FUNC were not unset.
This is very strange since this is explicitly done at the end of G_EXEC, there is no path around this, and the very same case with Nextcloud install works without this issue. Also the next G_EXEC calls during ownCloud install have the variables unset.
I made a lot of tests in console and couldn't find any way how the variable and the function are able to survive the G_EXEC call. Also the very same construction worked for with G_ERROR_HANDLER and it works well with G_EXEC in all other cases. What is different with the ownCloud install?


🈴 Simple unset two times does not help


🈯️ Found it, there was an obsolete tee pipe in place which was a prior attempt to handle occ command stack trace errors, before G_EXEC_POST_FUNC was implemented. The problem with pipes is, that pipe input functions (left side of |) loose any possibility to affect the originating shell environment, e.g. cannot unset variables of functions anymore and also cannot kill the originating shell. The same was the reason for (already present with G_ERROR_HANDLER:

# - Log to file by redirecting to subshell instead of piping, else G_EXEC cannot exit the script via "kill -INT $$": https://github.com/MichaIng/DietPi/issues/3127
Run_Update &> >(tee $FP_LOG_TMP); wait $!

Final important result: G_EXEC must never be piped. G_EXEC_POST_FUNC was the required implementation to handle errors, visible in output without producing an error exit code.

+ DietPi-Software | Remove obsolete tee from ownCloud occ install, which causes an issue as well: #3440 (comment)
+ DietPi-Globals | Add info that G_EXEC must never be piped
+ DietPi-Globals | unset variables or functions explicitly. unset tries to unset the input arguments as variables first and only if this fails tries to unset a such named functions. Also there might be a function with the same name defined while the variable is not.
+ DietPi-Globals | Do not assign input arguments as array ($@) to a string variable but assign it as concatenated string directly ($*)
+ CHANGELOG | DietPi-Globals: G_RUN_CMD and G_ERROR_HANDLER have been removed and replaced with G_EXEC
@MichaIng MichaIng merged commit 840ac5f into dev Mar 29, 2020
@MichaIng MichaIng deleted the G_EXEC branch March 29, 2020 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DietPi-Globals | G_ERROR_HANDLER improvements
1 participant