I had a problem with no token being returned, which was due to OpenSSL not being enabled in my PHP by default (uncomment extension=openssl.so in php.ini) - I needed to resort to a dsm() to detect the problem however.

I think it would be good to have error checking on the http request (there is error checking on the decoded json data, but that doesn't catch http level issues). The attached patch should improve this logging, although it could use more testing - it may also be good to consider doing a drupal_set_error (without the specific error message, perhaps), so users know something has gone wrong and admins can check the logs.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker’s picture

Status: Needs review » Needs work
+++ b/tmgmt_microsoft.plugin.incundefined
@@ -197,9 +197,11 @@ class TMGMTMicrosoftTranslatorPluginController extends TMGMTDefaultTranslatorPlu
+      if (isset($tokenRequest->data)) {
+        $token = json_decode($tokenRequest->data);

If the error handling is "complete" and the response of Microsoft is always nice, this condition is always true if no error.

Thus move it below the early exit condition, unconditional. Code is then better readable.

Also note that the error check now accesses $token->error and it might not be available.

miro_dietiker’s picture

Status: Needs work » Needs review
FileSize
911 bytes

I'm confused! Was there ever a $token->error? Note $token is already $tokenRequest->data.
If possible, attached code might be OK. If not, just remove the $token->error case...

Owen Barton’s picture

I am not familiar with the Microsoft API - I was assuming $token->error was an used for application errors on their side. http://msdn.microsoft.com/en-us/library/hh454950.aspx suggests we should indeed check for this, but I am not sure where the definitive documentation for this is.