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

Update ConfigurationProvider.php #2984

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

richardmtheobald
Copy link

Issue #, if available:
{"message":"CSM 'port' value must be an integer!","line":24,"file":"/var/app/current/vendor/aws/aws-sdk-php/src/ClientSideMonitoring/Configuration.php","trace":"#0 /var/app/current/vendor/aws/aws-sdk-php/src/ClientSideMonitoring/ConfigurationProvider.php(187): Aws\ClientSideMonitoring\Configuration->__construct()\n#1 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(204): Aws\ClientSideMonitoring\ConfigurationProvider::Aws\ClientSideMonitoring\{closure}()\n#2 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(169): GuzzleHttp\Promise\Promise::callHandler()\n#3 /var/app/current/vendor/guzzlehttp/promises/src/RejectedPromise.php(42): GuzzleHttp\Promise\Promise::GuzzleHttp\Promise\{closure}()\n#4 /var/app/current/vendor/guzzlehttp/promises/src/TaskQueue.php(48): GuzzleHttp\Promise\RejectedPromise::GuzzleHttp\Promise\{closure}()\n#5 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(248): GuzzleHttp\Promise\TaskQueue->run()\n#6 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(224): GuzzleHttp\Promise\Promise->invokeWaitFn()\n#7 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(269): GuzzleHttp\Promise\Promise->waitIfPending()\n#8 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(226): GuzzleHttp\Promise\Promise->invokeWaitList()\n#9 /var/app/current/vendor/guzzlehttp/promises/src/Promise.php(62): GuzzleHttp\Promise\Promise->waitIfPending()\n#10 /var/app/current/vendor/aws/aws-sdk-php/src/ClientSideMonitoring/ConfigurationProvider.php(327): GuzzleHttp\Promise\Promise->wait()\n#11 /var/app/current/vendor/aws/aws-sdk-php/src/ClientSideMonitoring/AbstractMonitoringMiddleware.php(271): Aws\ClientSideMonitoring\ConfigurationProvider::unwrap()\n#12 /var/app/current/vendor/aws/aws-sdk-php/src/ClientSideMonitoring/AbstractMonitoringMiddleware.php(176): Aws\ClientSideMonitoring\AbstractMonitoringMiddleware->unwrappedOptions()\n#13 /var/app/current/vendor/aws/aws-sdk-php/src/ClientSideMonitoring/AbstractMonitoringMiddleware.php(97): Aws\ClientSideMonitoring\AbstractMonitoringMiddleware->isEnabled()\n#14 /var/app/current/vendor/aws/aws-sdk-php/src/ClientResolver.php(605): Aws\ClientSideMonitoring\AbstractMonitoringMiddleware->__invoke()\n#15 /var/app/current/vendor/aws/aws-sdk-php/src/Middleware.php(96): Aws\ClientResolver::Aws\{closure}()\n#16 /var/app/current/vendor/aws/aws-sdk-php/src/Middleware.php(80): Aws\Middleware::Aws\{closure}()\n#17 /var/app/current/vendor/aws/aws-sdk-php/src/IdempotencyTokenMiddleware.php(77): Aws\Middleware::Aws\{closure}()\n#18 /var/app/current/vendor/aws/aws-sdk-php/src/AwsClientTrait.php(64): Aws\IdempotencyTokenMiddleware->__invoke()\n#19 /var/app/current/vendor/aws/aws-sdk-php/src/AwsClientTrait.php(58): Aws\AwsClient->executeAsync()\n#20 /var/app/current/vendor/aws/aws-sdk-php/src/AwsClientTrait.php(77): Aws\AwsClient->execute()\n#21 /var/app/current/consumer.php(555): Aws\AwsClient->__call()\n#22 {main}"}

Description of changes:
csm port requires an integer and will throw an error if a non-integer is provided

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

csm port requires an integer
@@ -182,7 +182,7 @@ public static function ini($profile = null, $filename = null)
}

// port is optional
if (empty($data[$profile]['csm_port'])) {
if (empty($data[$profile]['csm_port']) || filter_var($data[$profile]['csm_port'], FILTER_VALIDATE_INT) === false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be multi-line:

Suggested change
if (empty($data[$profile]['csm_port']) || filter_var($data[$profile]['csm_port'], FILTER_VALIDATE_INT) === false) {
if (empty($data[$profile]['csm_port'])
|| filter_var($data[$profile]['csm_port'], FILTER_VALIDATE_INT) === false
) {

@stobrien89
Copy link
Member

Thanks for the contribution @richardmtheobald. I left one comment about making the statement multi-line. Would you be able to add a unit test for this in Aws\Test\ClientSideMonitoring\ConfigurationProviderTest?

@richardmtheobald
Copy link
Author

I apologize, @stobrien89, but no thank you. The extent of my free labor for a company worth $1.8 trillion was the two hours it took to figure out why upgrading to a newer version of the AWS PHP SDK caused my software to suddenly start throwing errors. The only reason I submitted this pull request was to potentially prevent this issue from happening for someone else in the future. If Amazon does not like the formatting or requires a unit test, they are welcome to make any changes they so desire. Alternatively, I am available for $120 USD per hour with a minimum billing time of 2 hours. Thank you for your time. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants