The hook created on line 731 with the following call:

drupal_alter('rest_server_execute_errors', $error_alter_array, $controller, $arguments);

will never have any values set for $controller and $arguments because those variables do not exist within the handleException method on the RestServer object.

A potentential solution is to add two protected properties to the RESTServer object, set those properties to the values of the $controller and $argument in the error handling routine located in the last few lines of the RESTServer::handle method as follows:

    try {
      $result = services_controller_execute($controller, $arguments, $options);
    }
    catch (ServicesException $e) {
      $this->controller = $controller;
      $this->arguments = $arguments;
      $result = $this->handleException($e);
    }

and pass those properties as the third and fourth paramaters to the drupal_alter call as follows:

drupal_alter('rest_server_execute_errors', $error_alter_array, $this->controller, $this->arguments);

Comments

ygerasimov’s picture

Status: Active » Needs review
StatusFileSize
new1004 bytes

Good catch. We use handleException in rest_server_server() and we do not have controller and arguments by then. For example when in exception when controller cannot be resolved. So solution for this case is simply pass both variables to handleException method. See attached patch.

lisa.rae’s picture

Status: Needs work » Needs review
StatusFileSize
new1.86 KB

ok, that's pretty much the same patch I started with. I've worked up another patch, which I'm submitting now, that sets two new properties on the RESTServer class, and sets the values of $controller and $argument from inside RESTServer::handle, making them available when the error handler is triggered by an external call in rest_server_server().

I've tested this several times with different errors, and so far this has tested out fine, with values set and available when applicable.

Patch is attached.

Status: Needs review » Needs work

The last submitted patch, 1965016-2-controller-arguments-handleException.patch, failed testing.

lisa.rae’s picture

Version: 7.x-3.x-dev » 7.x-3.3
Status: Needs review » Needs work
StatusFileSize
new1.86 KB

Mmmm. Patched against the current stable version.

Submitting again.

marcingy’s picture

Version: 7.x-3.3 » 7.x-3.x-dev

Patches need to be against dev

marcingy’s picture

Status: Needs work » Reviewed & tested by the community

Patch from Yuriy looks good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1965016-2-controller-arguments-handleException.patch, failed testing.

lisa.rae’s picture

Version: 7.x-3.x-dev » 7.x-3.3
Status: Needs work » Needs review
StatusFileSize
new1.86 KB

ok, let's try this again with no interruptions, and set the patch to be tested :)

Status: Needs review » Needs work

The last submitted patch, 1965016-2-controller-arguments-handleException.patch, failed testing.

marcingy’s picture

Version: 7.x-3.3 » 7.x-3.x-dev
Status: Needs work » Reviewed & tested by the community

Please do not change versions. As previous comment patch from Yuriy is good.

ygerasimov’s picture

Status: Reviewed & tested by the community » Fixed

Marc, thank you for the review. I have committed patch #1

@lhridley, could you please check dev version of the module and confirm that bug is fixed?

lisa.rae’s picture

This appears to be working fine. I've not generated an error that didn't display the applicable controller so far.

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Adding potential solution to issue