makeRequest effectively ignores error responses.

[pnomolos]

    } catch(CTCTException $e) {
        $e->generateError();
    }
    return $return;

Right at the end of Authentication.php. Errors aren't thrown up the stack, so if anything goes wrong an error is generated but the library goes on acting if nothing happened. As of right now, this means if someone submits incorrect authentication information it's impossible to do anything without simplexml_load_xml($response['xml']) throwing an error - there's no way to tell whether or not a user's actually authenticated! This carries over to if anything goes wrong - it gets passed back as if it's valid information and there's no way for a developer to verify whether or not a call executed successfully.

[pnomolos]

My recommendation to fix this would be to throw the exception after the $e->generateError() call. I'd also recommend removing the echo $msgPrefix.' '.$this->getMessage().'<br />'; call in CTCTException->generateError() as displaying such messages should be left up to the developer.

[ryandavis84]

Hey,

I will take a look at this and make the necessary edits.

Thank you for your feedback.
Ryan Davis
Support Engineer

[mattsawyer77]

I'm having this exact same issue with the latest version of this code, so the issue is definitely not fixed yet.

[focorunner]

As written, the generateError function echoes errors, but since it returns nothing, the wrapper can just hang. If you alter generateError to return errors, those returned to makeRequest go nowhere because there is no code there to get the returned error and add it to the object returned by makeRequest to your calling function. I didn't write the lobrary, but I agree that current http request error status codes could be handled better. There are some simple edits I can make next week so that an httpError value is set and returned instead. In short, have the generateError function return the error rather than echo it, then capture it in makeRequest where generateError is called and add it to the object returned by makeRequest, so you can see if the http_error key is set and process any error response as you like. This should provide the response back to your function for handling, rather than echoing it, or returning it and crashing due to the returned object (from makeRequest) being empty. Once I can get to it, I'll add an update here. If you want to contribute a possible solution, feel free to make a pull request, also, as your solution may be better yet. Cheers.

[focorunner]

Quick Update. I have a solution I'm testing for this, but it would require changes in integrations that already include the existing wrapper library. My role has changed, but we'll try to get an updated test branch up with a quick reference to describe the response object passed up to application scripts on success vs. error. No promises on timeframe, as priorities are on other development projects that may provide other solutions for this issue.

[shaneiseminger]

I've run into this problem as well. My suggestion is to just throw exceptions as necessary, rather than catching and logging them and letting the code continue as if the error didn't occur.

My code is also trying to catch exceptions and send them to a different logging facility, but all I've been getting is the simplexml exception, because the real exception is being masked. Here's my quick workaround:

class CTCTException extends Exception{
    public function __construct($message, $code = 0, Exception $previous = null){
        parent::__construct($message, $code);
    }

    public function generateError($msgPrefix=null){
        throw $this;
        // $this->logError($this->message);
        // echo $msgPrefix.' '.$this->getMessage().'<br />';
    }

[ ... ]

Exception handling should be done by the implementing developer anyway.

[WoogieNoogie]

We've stopped most development on this wrapper with the release of new new SDK built off our AppConnect 2 endpoints. This new version handles errors very efficiently, and allows the developer to decide what to do with the exceptions.

[shaneiseminger]

Thanks, I'll look into that. It sure would have been helpful to know about this when I was working on ConstantContact integration, because then I wouldn't have integrated an outdated library. It would be helpful if your developer site linked to the new wrapper instead of the old one:

http://community.constantcontact.com/t5/Documentation/Code-Samples/ba-p/25019

Maybe the old wrapper could have a note in the GitHub readme saying it's deprecated too.

[WoogieNoogie]

You bring up a very good point, and I'm sorry for the confusion on that. We've switched over our Developer site to the new v2 information, but the community still contains some traces of the v1 information.

We'll go through and update the topics that we can, and I'll definitely add some info to our v1 repos forwarding to the new ones.