hellosign / hellosign-java-sdk

A Java SDK for the HelloSign API.
MIT License
13 stars 27 forks source link

api timeout setting #47

Open armyofda12mnkeys opened 7 years ago

armyofda12mnkeys commented 7 years ago

Noticed today it took minute(s) to connect to hellosign from our servers via the java sdk code I have (maybe issue from our end or your end, not sure yet)... Is there anyway to set a timeout manually with a method of pretend 10seconds so I can control what happens if taking too long/times-out?

cmpaul commented 7 years ago

Hi @armyofda12mnkeys! We are currently experiencing degraded performance due to an AWS issue (see https://status.hellosign.com/).

You're right that the timeout should be configurable however, I'm marking this as an enhacement.

armyofda12mnkeys commented 7 years ago

Awesome. do you think it will throw a subclass exception like 'HellosignTimeoutException' so kinda related to issue #45 (not sure if it would be the same 404 Exception or slightly different)?

cmpaul commented 7 years ago

@armyofda12mnkeys Sounds like we can do two things:

  1. Provide a configurable timeout for requests.
  2. Throw a specific exception to indicate the timeout was hit.

For item 2, I may bundle that with issue #46 (handle 409 responses). Would a new exception class be preferable to a new method on HelloSignExcepion, such as getResponseCode()?

armyofda12mnkeys commented 7 years ago

Can an Exception sometimes not be tied to a response code? like maybe you get a 200 OK response from Hellosign api to your java sdk ... but its actually an api json response that says { hellosign_exception_code: 105, 'HellosignMaintenanceException', 'Hellosign is undergoing maintenance updates' } which to me in the java code should throw a more detailed exception I can catch not reliant on me checking for pure responseCodes like 404/409 (so you'd then throw a HellosignMaintenanceException subclassed from HellosignException once you hear back from api and I can handle that specifically or the main HellosignException parent class [or interface?] if i dont care about specifically handling certain errors).

Just an idea, wasn't sure if that makes sense or breaks things.

EDIT: Im not sure the actual implemention of extending the error ... subclass/interface or just a enum that defines the types which pass into the Exception to define it which user can , like here are my own personal Exceptions related to our app tied to Hellosign (not sure if this is good code from my end but it works for our own errors hehe [Im more of a web developer]):


public enum OurOwnHellosignExceptionType {

  ESIGN_DEFAULT_UNKNOWN_EXCEPTION(0, "ESIGN_DEFAULT_UNKNOWN_EXCEPTION", "Unknown Error ... "),
  ESIGN_API_RECREATION_LIMIT(1, "ESIGN_API_RECREATION_LIMIT", "Number of e-signature API recreations within screener reached."),
  ESIGN_ALREADY_SIGNED(2, "ESIGN_ALREADY_SIGNED", "You have already signed the e-signature."),
  ESIGN_CALLCENTER_EMAIL_RECREATION_LIMIT(3, "ESIGN_CALLCENTER_EMAIL_RECREATION_LIMIT", "Number of e-signature emails within callcenter reached."),
  ESIGN_CALLCENTER_EMAIL_ALREADY_SENT_FOR_SITE(4, "ESIGN_CALLCENTER_EMAIL_ALREADY_SENT_FOR_SITE", "Email already sent to user in CallCenter.");

  private final int intErrorCode;
  private final String stringErrorCode;
  private final String errorDescription;

  private OurOwnHellosignExceptionType(int intErrorCode, String stringErrorCode, String errorDescription) {
    this.intErrorCode = intErrorCode;
    this.stringErrorCode = stringErrorCode;
    this.errorDescription = errorDescription;
  }

  public int getIntErrorCode() {
     return intErrorCode;
  }
  public String getStringErrorCode() {
     return stringErrorCode;
  }

  public String getErrorDescription() {
     return errorDescription;
  }

  @Override
  public String toString() {
    return String.valueOf(intErrorCode) + ", " + stringErrorCode +", "+ errorDescription;
  }
}

public class OurOwnHellosignException extends Exception {

    private static final long serialVersionUID = 1L;
    private String errorCode = "UNKNOWN_EXCEPTION";

    public OurOwnHellosignException () {
    }

    public OurOwnHellosignException (OurOwnHellosignExceptionType t) {
        super(t.getErrorDescription());
        this.errorCode = t.getStringErrorCode();
    }
    public OurOwnHellosignException (String message) {
        //TODO: maybe set the default type to that ESIGN_DEFAULT_UNKNOWN_EXCEPTION
        super (message);
    }
    public OurOwnHellosignException (String errorCode, String message) {
        super (message);
        this.errorCode = errorCode;
    }

    public OurOwnHellosignException (Throwable cause) {
        super (cause);
    }

    public OurOwnHellosignException (String message, Throwable cause) {
        super (message, cause);
    }
    public OurOwnHellosignException (String errorCode, String message, Throwable cause) {
        super (message, cause);
        this.errorCode = errorCode;
    }

    public String getErrorCode(){
        return this.errorCode;
    }
}
cmpaul commented 7 years ago

@armyofda12mnkeys Thanks for the explanation. I'd like to avoid having you write your own exception handling for server maintenance or timeouts. The way I envision it, you should only ever have to worry about one type of exception (HelloSignException) when doing anything with the SDK. That way you can just handle any error and not have it impact your application. That's one reason why I want to implement HTTP retries in the SDK

armyofda12mnkeys commented 7 years ago

yah, i was just giving an example our own Exceptions (related to Hellosign but separate than yours to give an example how i use simple enums to define our Exception types)... Like if a user tries to get an esign 5 times in a row in our public-facing app (using up 5 api requests) ... we allow the 1st 5 so if user modified data then shows up prefilled in a new esign presented to them... but if >5, we throw our own ESIGN_API_RECREATION_LIMIT exception and i handle it by showing the last signature-id(without need to recreate the esign waisting a 'full/heavy' api request that counts against our limit).

yes def have the timeouts/etc in your own Exception code. Just giving example of my own simple exceptions.

cmpaul commented 7 years ago

Ah, makes sense, thanks for the example! That will be helpful. :)

armyofda12mnkeys commented 7 years ago

Hey @cmpaul , just curious if there has been progress on this front and when think it can get in.

I did see another outage (Monday from 4-5pm EST and some maintenance updates the other day) so would be very interested in this 'timeout setting and returns nice Exceptions' feature.

cmpaul commented 7 years ago

@armyofda12mnkeys No progress yet, let me prioritize this. I'm also still trying to gauge whether I can bundle this as part of a larger move to something like OkHttp. Originally I had wanted to keep this SDK lean and mean, but that limits the kind of functionality we can add (like this!).

latoyazamill commented 4 years ago

@jspaetzel Do we have any plans to resolve this in the near future? We have a customer asking about this capability.

jspaetzel commented 4 years ago

@latoyazamill not on my roadmap at the moment

codylerum commented 3 years ago

This became an issue today when it took minutes for requests to timeout.

codylerum commented 3 years ago

At first glance the implementation utilizes java.net.URLConnection which supports a connect and read timeout.