brsanthu / google-analytics-java

Java API for Google Analytics Measurement Protocol (part of Universal Analytics).
134 stars 67 forks source link

Exception handling #44

Open IvanRF opened 7 years ago

IvanRF commented 7 years ago

https://github.com/brsanthu/google-analytics-java/blob/8ad1c1db9e6f148cff22ef95b1ea7d5e8f876bd4/src/main/java/com/brsanthu/googleanalytics/httpclient/ApacheHttpClientImpl.java#L100-L112

There is no way of knowing what happened if a post() fails. I think you should throw the error and let the caller deal with it. Other option is to set the Exception in a new field on the HttpResponse (which will always be empty in case of a failure).

Check:

robertvazan commented 6 years ago

I would just add that logging is only one of several possible ways to handle errors. And it's not the best solution here, because random network failures create a lot of log noise. I would prefer to be able to count the exceptions instead and raise alarm only if there are too many of them.

robertvazan commented 6 years ago

As for the actual API, I recommend throwing an exception in synchronous calls and storing it in the returned Future in asynchronous calls.

If that doesn't agree with you, then the next best option is to supply exception handler in config. Default handler would implement the old behavior. People could just rethrow in the handler if they prefer the exception to be propagated.

brsanthu commented 5 years ago

Added this api. Any feedback welcome. https://github.com/brsanthu/google-analytics-java/pull/61/commits/fb392c2e1155f4648fb3c180bc2b26c8fde1391c

robertvazan commented 5 years ago

Looks good. Could be simpler if you just propagated the exception.

robertvazan commented 5 years ago

The code reminds me of my own library NoException, which provides many different kinds of exception handlers. It is nevertheless trivial to adapt NoException handlers to the GoogleAnalyticsExceptionHandler interface.

brsanthu commented 5 years ago

NoException seems like a nice library but I would be hesitant to add another dependency. But hope functional interface would make it easy to adopt.