digitalfondue / vatchecker

java library for calling the eu vat checker web service
Apache License 2.0
20 stars 3 forks source link

Error handling if VAT Service is unavailable #2

Closed machajdik closed 4 years ago

machajdik commented 4 years ago

When unavailable, the VAT Service returns:

`

soap:Server MS_UNAVAILABLE ` There should be some error handling or retry for this case and other SOAP Faults.
syjer commented 4 years ago

hi @machajdik , you are right.

I'll add a 2 new properties (boolean error, String errorMessag) in https://github.com/digitalfondue/vatchecker/blob/30a3186f0b7a2867e950a60370c166d0eb9d668a/src/main/java/ch/digitalfondue/vatchecker/EUVatCheckResponse.java#L18 and handle it in a more specific way in https://github.com/digitalfondue/vatchecker/blob/master/src/main/java/ch/digitalfondue/vatchecker/EUVatChecker.java#L233 .

What do you think? Or do you prefer that the library launch an exception?

machajdik commented 4 years ago

Wow, thanks for your super-fast answer.

Both options are fine for me. Personally I would prefer a dedicated exception that includes the soap fault code and fault string.

Also, we have seen in our application that after some retries, the call ends up successsfully most of the time. So another option could be to retry (and maybe to have the retry count configurable).

syjer commented 4 years ago

Personally I would prefer a dedicated exception that includes the soap fault code and fault string.

On one side, I would agree with you, but it would break the retro-compatibility, as currently it report the result as simply "invalid". I guess I'll need to think a little bit more how to implement it, maybe I can support both way :)

Also, we have seen in our application that after some retries, the call ends up successsfully most of the time. So another option could be to retry (and maybe to have the retry count configurable).

I would avoid adding some kind of retry policy inside the library, I think it should be the job of an external library like resilience4j/spring retry & co.

machajdik commented 4 years ago

On one side, I would agree with you, but it would break the retro-compatibility, as currently it report the result as simply "invalid".

Well that's right. I guess its absolutely fine to have the 2 new properties.

machajdik commented 4 years ago

Some more information about possible fault strings (from wsdl):

In case of problems, the returned FaultString can take the following specific values:

  • INVALID_INPUT: The provided CountryCode is invalid or the VAT number is empty;
  • GLOBAL_MAX_CONCURRENT_REQ: Your Request for VAT validation has not been processed; the maximum number of concurrent requests has been reached. Please re-submit your request later or contact TAXUD-VIESWEB@ec.europa.eu for further information": Your request cannot be processed due to high traffic on the web application. Please try again later;
  • MS_MAX_CONCURRENT_REQ: Your Request for VAT validation has not been processed; the maximum number of concurrent requests for this Member State has been reached. Please re-submit your request later or contact TAXUD-VIESWEB@ec.europa.eu for further information": Your request cannot be processed due to high traffic towards the Member State you are trying to reach. Please try again later.
  • SERVICE_UNAVAILABLE: an error was encountered either at the network level or the Web application level, try again later;
  • MS_UNAVAILABLE: The application at the Member State is not replying or not available. Please refer to the Technical Information page to check the status of the requested Member State, try again later;
  • TIMEOUT: The application did not receive a reply within the allocated time period, try again later.
syjer commented 4 years ago

@machajdik , I've committed https://github.com/digitalfondue/vatchecker/commit/5b97fff536198de8dd9981e0558338f3517f1245 .

If it's ok for you, I'll do a release and push on maven central :).

Obviously, if you have any tips for improving the code, feel free :)

machajdik commented 4 years ago

Sure, great! Code looks good!

syjer commented 4 years ago

@machajdik the release is available on maven central now: https://repo1.maven.org/maven2/ch/digitalfondue/vatchecker/vatchecker/1.4.3/ :)

machajdik commented 4 years ago

Tested and found good. Thank you so much!

syjer commented 4 years ago

thank you for opening the issue :+1: