search

VisualAppeal / Matomo-PHP-API

PHP wrapper for the Matomo API.
MIT License
95 stars 33 forks source link

Feature request: more meaningful error codes and information when a request fails #49

Closed theteknocat closed 1 year ago

theteknocat commented 1 year ago

We have a finicky client server to work with that frequently returns a 404 not found. However, when using the API there's no way to report or log this, because any and all invalid response errors just seem to have code 11 (for empty response) and no information at all from the response itself.

The Response object provided by the Httpful library contains everything that's needed, it's just not being used in a helpful way. The _finishResponse method simply throws an InvalidResponseException with the return value of the _isValidResponse method along with the parsed out URL. It would be a lot more helpful if that exception message at least contained the response code from the request. To that end, the _finishResponse method should have the $buffer variable passed into it directly to allow for more robust response validation. That way _isValidResponse could be sent the full response object in order to do more robust response validation, returning the appropriate information in case it is invalid. This could then be used when throwing the exception in the _finishResponse method.

thelfensdrfer commented 1 year ago

I am happy to accept pull requests!

theteknocat commented 1 year ago

Ok great! My current client has lots of budget and has a finicky Matomo server that sometimes produces things like 404 errors and that’s when I have issues with the error output from this library. So I will look at forking it, implementing a solution and sending you a pull request sometime in the next couple of weeks.

theteknocat commented 1 year ago

What I can’t quite figure out, though, is that if there’s an error like being unable to resolve the domain (an issue with their VPN) then it does provide useful output. But I assume in that case it’s because httpful must be throwing an exception in cases like that. I’ll have to setup a dummy server with Postman or something in order to test different responses and how to handle them.

theteknocat commented 1 year ago

Ok, pull request #50 has been submitted for your review @thelfensdrfer.

Works great in my Laravel application so far. Look forward to your feedback on it.

theteknocat commented 1 year ago

@thelfensdrfer just pinging you in case you haven't seen this yet :). I'm sure you're busy with other things and life. It would be really great if you have time to review it in the next couple of weeks. I'd love to include this improvement with the code we're shipping to the client by the end of March.

thelfensdrfer commented 1 year ago

Sorry but I completely forgot about this issue. But it should be integrated now because we switched to guzzlehttp.

theteknocat commented 1 year ago

Oh that's awesome to hear! I hadn't got around to it anyway, obviously.

Next time I'm working on the project that was using this library I'll make sure to update it.