SSLMate / sslmate

The SSLMate Client - Buy and Manage SSL Certs from the Command Line
Other
99 stars 9 forks source link

Fix reissue and renew commands returning error status code on success #17

Closed GUI closed 9 years ago

GUI commented 9 years ago

When reissue or renew were called, the command line tool was exiting with a status code of 1, even if the command successfully completed. Since this is considered a failure, this made it a little tricky to write wrapper scripts around these commands. This fix ensures these commands exit with a successful exit code of 0 if they were actually successful.

(This could also be fixed by having do_wait_for_cert itself return 0, but it seemed like having the command_* methods explicitly return 0 was more consistent with how the other command_* methods are currently implemented.)

AGWA commented 9 years ago

Thanks! This has been merged.

By the way, the reissue that failed an hour ago was because of an undocumented error from our upstream that we'd never seen before. There's now logic in the backend to automatically retry the reissue when that error occurs.

GUI commented 9 years ago

Thanks for the quick merge! And I appreciate the reissue explanation! I had chalked that up to gremlins, but that's fantastic to know you saw the error and already have a fix in place. Wow!

Oh, and I forgot to note that this little fix was againt the apiv2 branch, because that's what I'm currently using, and where I happened to be in the code. Hope that's okay. However, it looks like the same issue might also affect master. If you'd like a separate pull request against master to fix this little issue there, just let me know (or it's also probably easy enough to to replicate this simple 2-line fix :).

Thanks again!

AGWA commented 9 years ago

API errors are religiously monitored. I want SSLMate to be as reliable as possible. It's a challenge given that our upstreams are kind of flaky, but retrying failed requests and having multiple redundant upstreams helps get the job done.

Good point about the master branch; I just fixed it in 582262cb49203efcd0390dceff6efab26d5f01ef. Thanks again for reporting this!