throw first exception when retrying requests fails

hfaulds commented 8 years ago

When performing bad queries rets servers often log you out.

I've been burned quite often where I have an invalid query but the exception I see is an authorization exception from one of the retries.

It makes more sense to me to raise the first exception when retrying queries.

summera commented 8 years ago

@hfaulds I noticed this as well and did some digging over the weekend. It started to happen after https://github.com/estately/rets/pull/136

We didn't see this before that change because we were setting @capabilities to nil after each retry. This would cause the client to re-login before each retry.

The best way to handle this, in my opinion, is resetting capabilities in #clean_setup. Or having the client re-login before each retry in another manner. Otherwise, the retries will be pointless because we won't be logged in. This will also fix the wrong exception part of the problem.

def clean_setup
   ...
  @capabilities = nil
end

hfaulds commented 8 years ago

@summera I'm open to the idea, I just wondering whether we'll potentially create excessive logins if we login after every error (at the moment we login again if we receive a not logged in error). Is it part of the RETS spec to log users out after an error, or is it just something some RETS servers do?

summera commented 8 years ago

@hfaulds That's a good question. I'm not sure whether it's part of the spec or not, but I have witnessed this behavior on multiple RETS server.

I can see in your case, where you just want to receive the correct exception, that re-logging in is not necessary. However, if you're in a situation where you want to make subsequent requests to correct your initial request with the same Rets::Client instance, like in a console or CLI, then re-logging in is necessary. And it's not always obvious that you need to do so.

Some of these servers won't throw a "not logged in error" on the retries after the initial exception. I think that's the main problem, otherwise we could catch the exception and then re-login before making further requests. Maybe this is an option if we can come up with a good way to do so? For example, Paragon RETS servers throw a server error like this the following after the first failed attempt:

HTTP status: 412, body: <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"/>
<title>412 - Precondition set by the client failed when evaluated on the Web server.</title>
<style type="text/css">
<!--
body{margin:0;font-size:.7em;font-family:Verdana, Arial, Helvetica, sans-serif;background:#EEEEEE;}
fieldset{padding:0 15px 10px 15px;}
h1{font-size:2.4em;margin:0;color:#FFF;}
h2{font-size:1.7em;margin:0;color:#CC0000;}
h3{font-size:1.2em;margin:10px 0 0 0;color:#000000;}
#header{width:96%;margin:0 0 0 0;padding:6px 2% 6px 2%;font-family:"trebuchet MS", Verdana, sans-serif;color:#FFF;
background-color:#555555;}
#content{margin:0 0 0 2%;position:relative;}
.content-container{background:#FFF;width:96%;margin-top:8px;padding:10px;position:relative;}
-->
</style>
</head>
<body>
<div id="header"><h1>Server Error</h1></div>
<div id="content">
 <div class="content-container"><fieldset>
  <h2>412 - Precondition set by the client failed when evaluated on the Web server.</h2>
  <h3>The request was not completed due to preconditions that are set in the request header. Preconditions prevent the requested method from being applied to a resource other than the one intended. An example of a precondition is testing for expired content in the page cache of the client.</h3>
 </fieldset></div>
</div>
</body>
</html>

They don't make it obvious that they logged you out. Given that login requests are very light requests (unlike searching transactions) and that this is how the gem behaved before https://github.com/estately/rets/pull/136, it doesn't seem like excessive logins are a problem in this scenario. Unless you previously saw problems due to excessive logins?

hfaulds commented 8 years ago

@summera sorry for taking so long with this. You're right, I added this in https://github.com/estately/rets/issues/191

summera commented 8 years ago

@hfaulds all good. thanks for fixing! :)

estately / rets

throw first exception when retrying requests fails #183