Close socket immediately for fire and forget commands

[tylerkaraszewski]

Details

It seems we don't close the sockets for fire and forget commands until after we finish the commands. Let's close them right after we send the response.

Fixed Issues

https://github.com/Expensify/Expensify/issues/411326

Tests

2024-07-10T22:36:43.812029+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Set ASYNC NVP expensify_fixAccountDate
2024-07-10T22:36:43.814120+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Bedrock\Client - Starting a request ~~ command: 'SetNameValuePair' clusterName: 'auth' headers: '[authToken: '<REDACTED>' name: 'expensify_fixAccountDate' value: '2024-07-10 22:36:43' Connection: 'forget' priority: '500' logParam: '[email protected]' urlToNewDot: 'https://dev.new.expensify.com:8082/' shouldReadUsingThreads: '1' shouldHandleOnyxUpdates: '1' clientUpdateID: '-1' maxNumberOfUpdates: '500' requestID: 'HQsUhy' lastIP: '10.2.2.1' writeConsistency: 'ASYNC' timeout: '110000']'
2024-07-10T22:36:43.814936+00:00 expensidev2004 bedrock: xxxxxx (BedrockServer.cpp:2074) _acceptSockets [main] [dbug] Accepting socket from '127.0.0.1:49016' on port '0.0.0.0:4444'
2024-07-10T22:36:43.815010+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Closing socket after use
2024-07-10T22:36:43.823305+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [dbug] Auth - AuthRequest #2 command='Get' rVL='cardList'  
2024-07-10T22:36:43.823841+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Bedrock\Client - Starting a request ~~ command: 'Get' clusterName: 'auth' headers: '[cardID: '' includeAllDeactivated: '1' authToken: '<REDACTED>' returnValueList: 'cardList' idempotent: '1' logParam: '[email protected]' urlToNewDot: 'https://dev.new.expensify.com:8082/' shouldReadUsingThreads: '1' shouldHandleOnyxUpdates: '1' clientUpdateID: '-1' maxNumberOfUpdates: '500' requestID: 'HQsUhy' lastIP: '10.2.2.1' writeConsistency: 'ASYNC' priority: '500' timeout: '110000']'
2024-07-10T22:36:43.823942+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Bedrock\Client - APC fetch host configs ~~ 0: 'auth'
2024-07-10T22:36:43.824018+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Bedrock\Client - Possible hosts ~~ nonBlacklistedHosts: '[0: 'auth']'
2024-07-10T22:36:43.824098+00:00 expensidev2004 php-fpm: HQsUhy /api.php [email protected] !web! ?api? [info] Bedrock\Client - Opening new socket ~~ host: 'auth' cluster: 'auth' pid: '1102'

---

Internal Testing Reminder: when changing bedrock, please compile auth against your new changes

[iwiznia]

Looks good. Does this mean we don't need to change anything in the PHP side?
Can you please do a test in PHP of:

• Send a command with fire&forget
• Send another command without fire and forget
• Check it opened a new socket

[tylerkaraszewski]

Added test logs.

[johnmlee101]

reverting this since it created flakey tests here: https://expensify.slack.com/archives/C07J32337/p1720694871685509

[tylerkaraszewski]

I will investigate next week.

[iwiznia]

hmmmm did not read that massive thread, but my guess is: before we were basically waiting for the fire&forget command to finish whenever we did another call to that bedrock instance, with this change we are not, so some tests expect some data to be set, but it is sometimes not because the fire&forget command runs/finishes after the test checks for it.

[johnmlee101]

Yeah, that's basically the gist. We'll want to make sure our test suite is set beforehand, but the change itself should still be good