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

[4.19.1] SDK should never be printing/echoing data #47

Closed
shengslogar opened this issue Jan 17, 2024 · 3 comments
Closed

[4.19.1] SDK should never be printing/echoing data #47

shengslogar opened this issue Jan 17, 2024 · 3 comments

Comments

@shengslogar
Copy link

shengslogar commented Jan 17, 2024

SDKs should, in principle, be headless. bdc39a1, tagged 4.17.1, added an echo statement to RetrySender without any rationale provided in the changelog or commit message, perhaps an artifact left over from internal debugging?

Searching across the codebase yields the following instances of this:

echo($response->getPayload());

echo $ex->getCode() . "\n";

echo($response->getPayload());

Happy to provide an example explaining why this is causing issues in our codebase if it's not self-evident.

EDIT: if output is required, hooking into ClientBuilder's withDebug mode might be appropriate.

@shengslogar
Copy link
Author

A temporary non-ideal workaround is to capture and flush output using object buffering:

/** @var \SmartyStreets\PhpSdk\US_Autocomplete_Pro\Client $autocompleteClient */

\ob_start();
$autocompleteClient->sendLookup(...);
\ob_clean();

// Use results of lookup...

Must also take care to \ob_clean in the event of a SmartyStreets\PhpSdk\Exceptions\SmartyException.

@smartyeric
Copy link

Hi

It looks like those echo statements were indeed leftover from internal debugging. We've removed those, along with some leftover artifacts from the US_Autocomplete classes, in the most recent version (5.0.1).

@shengslogar
Copy link
Author

Excellent! Happy to see this package getting some love 🙌

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

No branches or pull requests

2 participants