Blesta Api Sdk Curl Error Handling Incorrect

[gutterboy]

I've been using the Blesta API in conjunction with the SDK; however I have recently noticed that the error handling it does with cURL is incorrect.

 

For example we have this code in blesta_api.php

private function submit($model, $method, array $args = array(), $action = "POST") {
	
	$url = $this->url . $model . "/" . $method . "." . self::$format;

	$this->last_request = array(
		'url' => $url,
		'args' => $args
	);

	if ($action == "GET") {
		$url .= "?" . http_build_query($args);
		$args = null;
	}

	$ch = curl_init();
	curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
	curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $action);
	curl_setopt($ch, CURLOPT_URL, $url);
	if ($args) {
		curl_setopt($ch, CURLOPT_POSTFIELDS, http_build_query($args));
	}

	curl_setopt($ch, CURLOPT_HTTPAUTH, CURLAUTH_BASIC);
	curl_setopt($ch, CURLOPT_USERPWD, $this->user . ":" . $this->key);
	//curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
	//curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, false);
	$response = curl_exec($ch);
	$response_code = curl_getinfo($ch, CURLINFO_HTTP_CODE);
	curl_close($ch);
	
	return new BlestaResponse($response, $response_code);
}

Then we have this code inside blesta_response.php

/**
 * @var string The raw response from the API
 */	
private $raw;
/**
 * @var int The HTTP response code from the API
 */
private $response_code;

/**
 * Initializes the Blesta Response
 *
 * @param string $response The raw response data from an API request
 * @param int $response_code The HTTP response code for the request
 */
public function __construct($response, $response_code) {
	$this->raw = $response;
	$this->response_code = $response_code;
}

/**
 * Returns the response from the request
 *
 * @return mixed A stdClass object representing the response returned from the request, null if no response returned
 */
public function response() {
	$response = $this->formatResponse();
	if (isset($response->response))
		return $response->response;
	return null;
}

/**
 * Returns the HTTP response code
 *
 * @return int The HTTP response code for the request
 */
public function responseCode() {
	return $this->response_code;
}

/**
 * Returns the raw response
 *
 * @return string The raw response
 */
public function raw() {
	return $this->raw;
}

/**
 * Returns all errors contained in the response
 *
 * @return stdClass A stdClass object representing the errors in the response, false if invalid response
 */
public function errors() {
	if ($this->response_code != 200) {
		$response = $this->formatResponse();

		if (isset($response->errors))
			return $response->errors;
		else {
			$error = new stdClass();
			$error->error = $response;
			return $error;
		}
	}
	return false;
}

/**
 * Formats the raw response into a stdClass object
 *
 * @return stdClass A stdClass object representing the resposne
 */
private function formatResponse() {
	return json_decode($this->raw);
}

Giving particular notice to the errors method, first off it does this:

if ($this->response_code != 200) {

Just because the response code is != 200 doesn't mean there was an error, a response code of `301` will still return a valid response and shouldn't be treated as a possible error.

 

Secondly, if there is an error it is not going to be contained within $this->raw; this will simply contain boolean false. The correct way to get errors would be via the function curl_error().

 

So with the above in mind, shouldn't it be done similar to:

 

Add the below line to the submit method in blesta_api.php

$response_error = curl_error($ch);

CHANGE:

return new BlestaResponse($response, $response_code);

TO:

return new BlestaResponse($response, $response_error, $response_code);

Then update blesta_response.php

 

ADD:

/**
 * @var int The cURL error (if any) from the API
 */
private $response_error;	

UPDATE the constructor..

public function __construct($response, $response_error, $response_code) {
	$this->raw = $response;
	$this->response_error = $response_error;
	$this->response_code = $response_code;
}

Then changing the errors method to something like this:

public function errors() {
	if ($this->response_code != 200 or $this->response_error) {
		$response = $this->formatResponse();

		if (isset($response->errors))
			return $response->errors;
		else {
			$error = new stdClass();
			$error->error = ($response) ? $response : $this->response_error;
			return $error;
		}
	}
	return false;
}

I had to allow for Blesta errors, which aren't cURL errors so I left in some of the code.

[gutterboy]

Slight edit to the errors method above, this would probably be the best way to handle it actually..

public function errors() {
	
	$response = $this->formatResponse();
	
	if (isset($response->errors) or $this->response_error) {
		
		if (isset($response->errors))
			return $response->errors;
		else {
			$error = new stdClass();
			$error->error = $this->response_error;
			return $error;
		}
		
	}
	
	return false;

}

[gutterboy]

I just posted this so the SDK could be improved and to give a heads up about the issue.