Number insights response unusable with non-zero status code?

ryan-mend commented 7 months ago

We've noticed that on the Vonage API Dashboard there lies a link to take you to a Getting started with Number Insight page here.

When we enter this number (18104967360) into the Getting started with Number Insight page and press the Look up number button, the look up appears to succeed and this gets displayed on the page:

However when trying to look that same number (18104967360) up via this SDK, because the status code coming back in the look up response is 44 (non-zero) an exception is thrown and the response is obfuscated from the consuming application. This behavior seems consistent in all versions of this SDK.

This leads me to a few questions/concerns:

Why would the php sdk library be written in such a way that you wouldn't be able to glean the same information about the number being looked up as is exposed on the number lookup page available from the api dashboard for the exact same number?
When the php sdk library lookup fails in this way I presume we are still charged or the lookup despite the exception being thrown and response being made unavailable to the application leveraging this sdk library?
If this exception throwing was by design, and not an oversight, is that indicative of the response of the lookup being unreliable and unusable? If that is true then why display the data like you do on the number lookup page available from the api dashboard?

https://github.com/Vonage/vonage-php-sdk-core/blob/28e056625c7c82a3a20fd13a91af396107afdcf9/src/Insights/Client.php#L119

ryan-mend commented 7 months ago

@SecondeJK Curious as to what the plan here is?

SecondeJK commented 7 months ago

The plan is to look into this, which is why it has been labelled with work-scheduled :)

ryan-mend commented 7 months ago

@SecondeJK Understood. I apologize for the confusion. I misunderstood what work-scheduled meant. My assumption was that it meant we know what we want to do just haven't done it yet. Thank you for the update.

SecondeJK commented 6 months ago

Hi @ryan-mend, just picking this up now. With regards to the implementation, it's clearly not right so I'll be be tinkering with the internals of the Client to get responses out. The dashboard UI tester is clearly configured here in a different way to the SDK where it knows what status will be charged and which will not, so I'll check these details.

I've also tested this to see if a charge is invoked with a 43 or 44 status and you're correct, it is still charged which is definitely a bug that needs fixing in the SDK.

SecondeJK commented 6 months ago

Fixed by #466, release coming shortly. Thanks for reporting this @ryan-mend, calls with partial status codes can now be retrieved.

ryan-mend commented 6 months ago

@SecondeJK Any chance you could make the same change to older versions of the sdk? We're currently forced to use no later version than 2.3 or 2.4 due to our version of php (7.3)?

SecondeJK commented 6 months ago

Sorry, this is something that I wouldn't do unless you submitted a PR for it. PHP7.3 was EOL with security support stopping 2 years ago so it's advocating against best security practise essentially

Vonage / vonage-php-sdk-core

Number insights response unusable with non-zero status code? #463