fix(installer): guard optional function calls that fatal on PHP 8.2#60
Conversation
Two unrelated installer-time crashes share the same root cause:
PHP's @ operator does not suppress 'undefined function' fatal errors,
and XOOPS's custom error handler ignores @ for warnings, so both
spots logged or fataled on environments where the called function
or backing file was absent.
- install/page_modcheck.php: @mysqli_get_client_info() crashed the
requirements page when the mysqli extension was not loaded, hiding
the very diagnostic meant to surface that condition. Guard with
function_exists() and pass an empty info string when missing;
xoDiag() still reports MySQLi as missing via the existing
function_exists('mysqli_connect') flag.
- xoops_lib/modules/protector/class/protector.php: the constructor
read its configcache via @file_get_contents() before the cache
was ever written. On a fresh install the cache file does not
exist until postcheck_functions.php calls updateConfFromDb(),
and every page in between logged a warning to log.txt because
XoopsLogger's error handler does not honour @. Guard with
is_file() and treat a missing cache as an empty serialized
payload; subsequent unserialize/empty checks already handle that
shape.
Reported against PHP 8.2.31 / MySQL 5.7.44 / Apache 2.4.66 on
Windows during 2.7.0 install testing.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis pull request replaces error suppression operators (@) in two separate initialization paths with explicit conditional checks. The installer's MySQLi client info now verifies function existence before calling it, and the protector's config cache now checks file existence before reading the cache file. ChangesError Suppression Removal
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #60 +/- ##
============================================
+ Coverage 18.04% 18.12% +0.08%
- Complexity 7845 7854 +9
============================================
Files 665 666 +1
Lines 43083 43136 +53
============================================
+ Hits 7775 7820 +45
- Misses 35308 35316 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@htdocs/install/page_modcheck.php`:
- Line 62: Remove the redundant error-suppression operator before
mysqli_get_client_info in the xoDiag call: since
function_exists('mysqli_get_client_info') already ensures the function exists,
delete the leading @ from `@mysqli_get_client_info`() so the line calls
mysqli_get_client_info() directly while keeping the existing guard and xoDiag
invocation intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6216c085-264d-4940-8ec5-f2c4968e5af0
📒 Files selected for processing (2)
htdocs/install/page_modcheck.phphtdocs/xoops_lib/modules/protector/class/protector.php
The function_exists('mysqli_get_client_info') guard already prevents the
undefined-function fatal, and the function emits no warnings in normal
operation, so the @ suppression on the call is dead weight. Removing it
also keeps both halves of this fix consistent (the protector.php change
has no @) and matches the PR's own rationale that @ is misleading here.
Addresses CodeRabbit review feedback on PR XOOPS#60.
|
Add Installer and Tests subsections under 2.7.0 Final for the work merged today: - #60 guard optional function calls that fatal on PHP 8.2 (mysqli_get_client_info(), Protector config cache) - #62 block install with a clear message when a mandatory PHP extension is missing, plus full required-extension guards on the DB steps - #63 unit coverage for the installer required-extension helpers
Two unrelated installer-time crashes share the same root cause: PHP's @ operator does not suppress 'undefined function' fatal errors, and XOOPS's custom error handler ignores @ for warnings, so both spots logged or fataled on environments where the called function or backing file was absent.
install/page_modcheck.php: @mysqli_get_client_info() crashed the requirements page when the mysqli extension was not loaded, hiding the very diagnostic meant to surface that condition. Guard with function_exists() and pass an empty info string when missing; xoDiag() still reports MySQLi as missing via the existing function_exists('mysqli_connect') flag.
xoops_lib/modules/protector/class/protector.php: the constructor read its configcache via @file_get_contents() before the cache was ever written. On a fresh install the cache file does not exist until postcheck_functions.php calls updateConfFromDb(), and every page in between logged a warning to log.txt because XoopsLogger's error handler does not honour @. Guard with is_file() and treat a missing cache as an empty serialized payload; subsequent unserialize/empty checks already handle that shape.
Reported against PHP 8.2.31 / MySQL 5.7.44 / Apache 2.4.66 on Windows during 2.7.0 install testing.
Summary by CodeRabbit