-
Notifications
You must be signed in to change notification settings - Fork 52
Currency API Layer Error Handling #3776
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,10 @@ | |
| namespace Modules\Invoice\Services; | ||
|
|
||
| use GuzzleHttp\Client; | ||
| use GuzzleHttp\Exception\ConnectException; | ||
| use GuzzleHttp\Exception\RequestException; | ||
| use Illuminate\Support\Facades\Cache; | ||
| use Illuminate\Support\Facades\Log; | ||
| use Modules\Invoice\Contracts\CurrencyServiceContract; | ||
|
|
||
| class CurrencyService implements CurrencyServiceContract | ||
|
|
@@ -17,13 +20,15 @@ public function __construct() | |
|
|
||
| public function setClient() | ||
| { | ||
| $headers = $headers = [ | ||
| $headers = [ | ||
| 'apikey' => config('services.currencylayer.access_key'), | ||
| ]; | ||
|
|
||
| $this->client = new Client([ | ||
| // 'base_uri' => 'http://apilayer.net/api', //This is old API URL | ||
| 'base_uri' => 'https://api.apilayer.com', // This is new API created from https://apilayer.com/marketplace/currency_data-api | ||
| 'headers' => $headers, | ||
| 'base_uri' => 'https://api.apilayer.com', | ||
| 'headers' => $headers, | ||
| 'timeout' => 10, | ||
| 'connect_timeout' => 5, | ||
| ]); | ||
| } | ||
|
|
||
|
|
@@ -48,35 +53,71 @@ public function getAllCurrentRatesInINR() | |
| private function fetchExchangeRateInINR() | ||
| { | ||
| if (! config('services.currencylayer.access_key')) { | ||
| return round(config('services.currencylayer.default_rate'), 2); | ||
| } | ||
| Log::warning('CurrencyLayer API key missing. Using default rate.'); | ||
|
|
||
| $response = $this->client->get('currency_data/live', [ | ||
| 'query' => [ | ||
| 'access_key' => config('services.currencylayer.access_key'), | ||
| 'currencies' => 'INR', | ||
| ], | ||
| ]); | ||
| return round(config('services.currencylayer.default_rate', 83.00), 2); | ||
| } | ||
|
|
||
| $data = json_decode($response->getBody()->getContents(), true); | ||
| try { | ||
| $response = $this->client->get('currency_data/live', [ | ||
| 'query' => [ | ||
| 'currencies' => 'INR', | ||
| 'source' => 'USD', | ||
| ], | ||
| ]); | ||
|
|
||
| $data = json_decode($response->getBody()->getContents(), true); | ||
|
|
||
| if (empty($data) || empty($data['quotes']['USDINR'])) { | ||
| throw new \Exception('Invalid API response structure.'); | ||
| } | ||
|
Comment on lines
+71
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix potential PHP notice in validation logic. The expression Apply this diff to fix the validation: - if (empty($data) || empty($data['quotes']['USDINR'])) {
+ if (empty($data) || !isset($data['quotes']['USDINR']) || empty($data['quotes']['USDINR'])) {
throw new \Exception('Invalid API response structure.');
}Alternatively, validate in stages: - if (empty($data) || empty($data['quotes']['USDINR'])) {
+ if (empty($data)) {
+ throw new \Exception('Empty API response.');
+ }
+
+ if (!isset($data['quotes']['USDINR']) || empty($data['quotes']['USDINR'])) {
throw new \Exception('Invalid API response structure.');
}🤖 Prompt for AI Agents |
||
|
|
||
| return round($data['quotes']['USDINR'], 2); | ||
| } catch (ConnectException $e) { | ||
| Log::error('Currency API connection failed: ' . $e->getMessage()); | ||
| } catch (RequestException $e) { | ||
| Log::error('Currency API request error: ' . $e->getMessage()); | ||
| } catch (\Throwable $e) { | ||
| Log::error('Unexpected error fetching exchange rate: ' . $e->getMessage()); | ||
| } | ||
|
|
||
| return round($data['quotes']['USDINR'], 2); | ||
| return round(config('services.currencylayer.default_rate', 83.00), 2); | ||
| } | ||
|
|
||
| private function fetchAllExchangeRateInINR() | ||
| { | ||
| if (! config('services.currencylayer.access_key')) { | ||
| return round(config('services.currencylayer.default_rate'), 2); | ||
| } | ||
| Log::warning('CurrencyLayer API key missing. Using default rate.'); | ||
|
|
||
| $response = $this->client->get('currency_data/live', [ | ||
| 'query' => [ | ||
| 'access_key' => config('services.currencylayer.access_key'), | ||
| ], | ||
| ]); | ||
| return [ | ||
| 'USDINR' => round(config('services.currencylayer.default_rate', 83.00), 2), | ||
| ]; | ||
| } | ||
|
|
||
| $data = json_decode($response->getBody()->getContents(), true); | ||
| try { | ||
| $response = $this->client->get('currency_data/live', [ | ||
| 'query' => [ | ||
| 'source' => 'USD', | ||
| ], | ||
| ]); | ||
|
|
||
| $data = json_decode($response->getBody()->getContents(), true); | ||
|
|
||
| if (empty($data) || empty($data['quotes'])) { | ||
| throw new \Exception('Invalid API response structure.'); | ||
| } | ||
|
|
||
| return $data['quotes']; | ||
| } catch (ConnectException $e) { | ||
| Log::error('Currency API connection failed: ' . $e->getMessage()); | ||
| } catch (RequestException $e) { | ||
| Log::error('Currency API request error: ' . $e->getMessage()); | ||
| } catch (\Throwable $e) { | ||
| Log::error('Unexpected error fetching all exchange rates: ' . $e->getMessage()); | ||
| } | ||
|
|
||
| return $data['quotes']; | ||
| return [ | ||
| 'USDINR' => round(config('services.currencylayer.default_rate', 83.00), 2), | ||
| ]; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Extract default rate to a helper method.
The default rate calculation
round(config('services.currencylayer.default_rate', 83.00), 2)is duplicated across lines 58, 84, 93, and 120. This violates the DRY principle.Extract this to a private helper method:
Then replace all other occurrences (lines 84, 93, 120) with
$this->getDefaultRate().🤖 Prompt for AI Agents