There are a few inconsistencies in how data is processed and then a response returned.

The doc for the process() method is as follows:

  /**
   * Processes data.
   *
   * @param object $data
   *  The encoded data
   *
   * @return object
   *   The unserialized data as a PHP object.
   *
   * @throws Webhook_Plugins_Processor_ErrorException
   */
  public function process(stdClass $data);

Firstly, there is no catching of Webhook_Plugins_Processor_ErrorException errors when process() is called in webhook_process_request(). You could catch and then return a nice status code.

Secondly, as webhook_process_request() is a menu callback, I would have expected a string to be the value returned. However, the way it is coded, the value coming back out of process() is an "object". Fair enough if multiple processors are to be strung together. webhook_process_request() could just return '' or an OK?

Thirdly, it seems that Webhook_Example_Plugins_Processor_GithubLog->process is returning a NULL rather than a $data object.

So you could do something like:

try {
  $processor->process($data);
  return 'ok';
}
catch (Webhook_Plugins_Processor_ErrorException $e) {
  drupal_add_http_header('Status', '400 Bad request');
  $content = $e->getMessage();
  print "<html><head><title>$status</title></head><body>$content</body></html>";
  drupal_exit();
}

As it stands my process functions are now doing their own response handling back to the browser. It would be nicer for them to throw the error.

Let me know if you want a patch.

Thanks for the module BTW. It is just the ticket for what I am doing.

Comments

skwashd’s picture

skwashd commented
Status: Active » Fixed

Thanks for the bug report.

I have implemented a more advanced version of what you suggested above. When an exception is thrown the thrower can decide the HTTP status code to use. This was committed as 406298e. It is assumed that the implementing class shouldn't output anything.

Status: Fixed » Closed (fixed)

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