feat: Added configuration options for error levels #394
Conversation
WalkthroughThe chaining factory has been updated to support two new optional configuration keys— Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
![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
🧹 Nitpick comments (1)
src/ProviderFactory/ChainFactory.php (1)
41-46: Consider validating log level values against PSR-3 standards.The configuration accepts any string value for log levels, which could lead to runtime errors if invalid values are provided. Consider adding validation to restrict values to valid PSR-3 log levels: emergency, alert, critical, error, warning, notice, info, debug.
🔎 Proposed validation in configureOptionResolver
$resolver->setDefault('geocode_log_level', null); $resolver->setAllowedTypes('geocode_log_level', ['string', 'null']); +$resolver->setAllowedValues('geocode_log_level', function ($value) { + return $value === null || in_array($value, ['emergency', 'alert', 'critical', 'error', 'warning', 'notice', 'info', 'debug'], true); +}); $resolver->setDefault('reverse_log_level', null); $resolver->setAllowedTypes('reverse_log_level', ['string', 'null']); +$resolver->setAllowedValues('reverse_log_level', function ($value) { + return $value === null || in_array($value, ['emergency', 'alert', 'critical', 'error', 'warning', 'notice', 'info', 'debug'], true); +});
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/ProviderFactory/ChainFactory.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/ProviderFactory/ChainFactory.php (2)
src/ProviderFactory/AbstractFactory.php (1)
getProvider(41-41)src/ProviderFactory/HostIpFactory.php (1)
getProvider(30-35)
🔇 Additional comments (3)
src/ProviderFactory/ChainFactory.php (3)
33-33: LGTM: Docblock accurately reflects the new configuration options.The updated docblock correctly documents the new optional parameters with appropriate types.
59-60: Configuration option looks good structurally.The default and type configuration are appropriate. See the previous comment for a recommendation to add value validation.
62-63: Configuration option looks good structurally.The default and type configuration are appropriate. See the previous comment for a recommendation to add value validation.
|
|
||
| if (isset($config['geocode_log_level'])) { | ||
| $provider->setGeocodeQueryLogLevel($config['geocode_log_level']); | ||
| } | ||
| if (isset($config['reverse_log_level'])) { | ||
| $provider->setReverseQueryLogLevel($config['reverse_log_level']); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its context
if [ -f "src/ProviderFactory/ChainFactory.php" ]; then
echo "=== File found, showing lines 30-55 for context ==="
sed -n '30,55p' src/ProviderFactory/ChainFactory.php
else
echo "File not found at src/ProviderFactory/ChainFactory.php"
fiRepository: geocoder-php/BazingaGeocoderBundle
Length of output: 967
🏁 Script executed:
# Check composer.json for the geocoder-php/chain-provider version
if [ -f "composer.json" ]; then
echo "=== composer.json content ==="
cat composer.json | jq '.require, .["require-dev"]' 2>/dev/null || grep -A5 -B5 "chain-provider" composer.json
fiRepository: geocoder-php/BazingaGeocoderBundle
Length of output: 2228
🏁 Script executed:
# Search for Chain provider class and method definitions in the codebase
echo "=== Searching for Chain class definition ==="
rg "class Chain" --type php -A 20 | head -50Repository: geocoder-php/BazingaGeocoderBundle
Length of output: 2878
🏁 Script executed:
# Look for method definitions related to log levels
echo "=== Searching for setGeocodeQueryLogLevel and setReverseQueryLogLevel ==="
rg "setGeocodeQueryLogLevel|setReverseQueryLogLevel" --type phpRepository: geocoder-php/BazingaGeocoderBundle
Length of output: 389
🌐 Web query:
geocoder-php/chain-provider github releases v4.5.0
💡 Result:
Do you want a link to the GitHub release page for geocoder-php/chain-provider v4.5.0, or just confirmation whether that tag/release exists?
🌐 Web query:
geocoder-php/chain-provider Chain class setGeocodeQueryLogLevel setReverseQueryLogLevel
💡 Result:
Short answer: The Chain provider does not define setGeocodeQueryLogLevel or setReverseQueryLogLevel. The official Chain provider only composes/iterates providers — it has no per-query log-level setters in its API. [1][2]
If you need to control logging per-query:
- Configure a PSR-3 logger on each underlying provider (or wrap providers) and control levels there.
- Or wrap the Chain provider with your own decorator that adjusts logging around geocodeQuery()/reverseQuery() calls.
Sources:
- Chain provider docs (describes Chain behavior, no log-level API). [1]
- Geocoder GitHub repository / docs (API overview; Chain is a provider that iterates providers). [2]
🏁 Script executed:
# Check if we can find more info about the Chain provider package
# Look for any vendor files or composer.lock that might help
if [ -f "composer.lock" ]; then
echo "=== Looking for chain-provider in composer.lock ==="
jq '.packages[] | select(.name == "geocoder-php/chain-provider") | {name, version, description}' composer.lock 2>/dev/null || grep -A 10 "chain-provider" composer.lock | head -20
fiRepository: geocoder-php/BazingaGeocoderBundle
Length of output: 60
🏁 Script executed:
# Search for Provider class definition and its methods
echo "=== Searching for Provider class methods ==="
rg "class Provider|function setGeocodeQueryLogLevel|function setReverseQueryLogLevel" --type php | head -30Repository: geocoder-php/BazingaGeocoderBundle
Length of output: 218
🌐 Web query:
Chain provider geocoder-php github.com willdurand/geocoder-php/chain-provider
💡 Result:
The chain provider (package geocoder-php/chain-provider) is a special Geocoder provider that takes a list/array of providers and iterates them in order, stopping when one returns a result. It’s provided in the Geocoder PHP project; example usage and docs are on the project site and GitHub. [1][2]
Sources:
[1] geocoder-php/Geocoder (GitHub) — Chain provider description.
[2] Geocoder PHP documentation — “The Chain Provider” (geocoder-php.org).
🏁 Script executed:
# Look at the directory structure to see if there are vendor files
echo "=== Checking directory structure for vendor or composer files ==="
ls -la | grep -E "(composer|vendor)"Repository: geocoder-php/BazingaGeocoderBundle
Length of output: 207
🌐 Web query:
site:github.com willdurand geocoder-php chain-provider Chain.php setGeocodeQueryLogLevel
💡 Result:
I searched the geocoder-php repositories and could not find any method named setGeocodeQueryLogLevel on the Chain provider (or a Chain.php defining that method). The Chain provider is documented as geocoder-php/chain-provider and used by the main Geocoder project and integrations (e.g. GeocoderLaravel). [1][2]
Sources:
- Geocoder (main repo) — docs mention the Chain provider. [1]
- GeocoderLaravel — default config uses the Chain provider. [2]
If you want, tell me the repository/path where you expect that method (or paste the file) and I’ll look for it more precisely.
🏁 Script executed:
# Try to find composer.lock and check the actual chain-provider version installed
if [ -f "composer.lock" ]; then
echo "=== Chain provider version in composer.lock ==="
python3 << 'PYEOF'
import json
try:
with open('composer.lock', 'r') as f:
data = json.load(f)
for pkg in data.get('packages', []) + data.get('packages-dev', []):
if pkg.get('name') == 'geocoder-php/chain-provider':
print(f"Package: {pkg['name']}")
print(f"Version: {pkg.get('version', 'N/A')}")
print(f"Source: {pkg.get('source', {}).get('url', 'N/A')}")
except Exception as e:
print(f"Error: {e}")
PYEOF
fiRepository: geocoder-php/BazingaGeocoderBundle
Length of output: 164
🏁 Script executed:
# Check the test file to see what it actually tests
echo "=== Checking ProviderFactoryTest for Chain-related tests ==="
grep -A 20 -B 5 "ChainFactory\|setGeocodeQueryLogLevel\|setReverseQueryLogLevel" tests/Functional/ProviderFactoryTest.php 2>/dev/null || echo "Pattern not found in test file"Repository: geocoder-php/BazingaGeocoderBundle
Length of output: 169
🏁 Script executed:
# Let's also check what methods Chain provider actually has by searching comments/docs
echo "=== Looking for any documentation or configuration related to Chain logging ==="
rg "Chain|geocode_log_level|reverse_log_level" --type php -B 2 -A 2 src/ | head -60Repository: geocoder-php/BazingaGeocoderBundle
Length of output: 3384
Remove the calls to non-existent methods on the Chain provider.
The setGeocodeQueryLogLevel() and setReverseQueryLogLevel() methods do not exist in the Chain provider API (geocoder-php/chain-provider v4.5.x). These method calls at lines 43-46 will cause a runtime error: "Call to undefined method". The Chain provider only iterates through child providers—it has no per-query log-level setter methods. Either remove this code or configure logging on individual child providers instead.
🤖 Prompt for AI Agents
In src/ProviderFactory/ChainFactory.php around lines 40 to 46, the code calls
setGeocodeQueryLogLevel() and setReverseQueryLogLevel() on the Chain provider
which do not exist and will throw "Call to undefined method"; remove those two
method calls from the Chain provider setup and, if per-query log levels are
required, iterate over the configured child providers and set their
geocode/reverse log levels individually (or apply logging configuration when
each child provider is created) instead.
|
You could cherry-pick commit from that PR to give the credit also to the author 😊 |
|
I am not sure how to do this. |
|
|
3 participants
#366
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.