Moodle
  1. Moodle
  2. MDL-32949

Web service exception should return errorcode (from moodle exception)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Web Services
    • Labels:
      None
    • Testing Instructions:
      Hide

      Dome clients: https://github.com/moodlehq/sample-ws-clients

      Run the REST demo client with json then xml. The second time you'll run it on core_user_create_users it should throw an exception as the user are already created. Check that both json/xml return errorcode.

      Run the XML-RPC and SOAP demo client, check that errorcode are also returned.

      Show
      Dome clients: https://github.com/moodlehq/sample-ws-clients Run the REST demo client with json then xml. The second time you'll run it on core_user_create_users it should throw an exception as the user are already created. Check that both json/xml return errorcode. Run the XML-RPC and SOAP demo client, check that errorcode are also returned.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      40062

      Description

      Add error code in REST/SOAP/...

      example REST-XML:

      <?xml version="1.0" encoding="UTF-8"?>
      <EXCEPTION class="moodle_exception">
          <ERRORCODE>missingcourse</ERRORCODE>
          <MESSAGE>This is a not translated message</MESSAGE>
          <DEBUGINFO>This is a not translated message with more detailled info</DEBUGINFO>
      </EXCEPTION>
      

      example SOAP change in webservice/soap/locallib.php:

      public function fault($fault = null, $code = "Receiver")
          {
              //intercept any exceptions with debug info and transform it in Moodle exception
              if ($fault instanceof Exception) {
                  //add the debuginfo to the exception message if debuginfo must be returned
                  if (debugging() and isset($fault->debuginfo)) {
                      $fault = new SoapFault($fault->errorcode, $fault->getMessage() . ' | DEBUG INFO: ' . $fault->debuginfo); //HERE IS THE CHANGE $fault->errorcode PS: NOT TESTED ;)
                  }
              }
      
              return parent::fault($fault, $code);
          }
      

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          I've added this to the MUST FIX for 2.3. It' blocking MDL-28557 and preventing us to have proper exception/warning handling ASAP.

          Show
          Eloy Lafuente (stronk7) added a comment - I've added this to the MUST FIX for 2.3. It' blocking MDL-28557 and preventing us to have proper exception/warning handling ASAP.
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Eloy can you have a look, two issues:

          a) XMLRPC use php exception faultcode. It means it's a long. What I did is to transform the errorcode into a unique digit and keep only 8 digits so the long is valid. Of course there can be collisions (but should be pretty rare) so I added the original errorcode into the message. I don't like to much having a faultcode not perfect. Should we remove it and force the client to parse the message or should we let it the client deal with the very rare collision? I would vote for always returning a faultcode set to 0.

          b) it seems to me that SOAP Zend library hardcode the faultCode values. So I ended adding the errorcode to the message and the client has no choice than parsing the message. Note that I could avoid to return parent::fault in moodle_zend_soap_server::fault and so return the same kind of "broken" faultCode as explain in a). But I voted for XMLRPC/SOAP returning 0/Receiver instead of the "broken" faultCode, waiting for your opinion.

          Show
          Jérôme Mouneyrac added a comment - - edited Eloy can you have a look, two issues: a) XMLRPC use php exception faultcode. It means it's a long. What I did is to transform the errorcode into a unique digit and keep only 8 digits so the long is valid. Of course there can be collisions (but should be pretty rare) so I added the original errorcode into the message. I don't like to much having a faultcode not perfect. Should we remove it and force the client to parse the message or should we let it the client deal with the very rare collision? I would vote for always returning a faultcode set to 0. b) it seems to me that SOAP Zend library hardcode the faultCode values. So I ended adding the errorcode to the message and the client has no choice than parsing the message. Note that I could avoid to return parent::fault in moodle_zend_soap_server::fault and so return the same kind of "broken" faultCode as explain in a). But I voted for XMLRPC/SOAP returning 0/Receiver instead of the "broken" faultCode, waiting for your opinion.
          Hide
          Jérôme Mouneyrac added a comment -

          Also need to integrate MDL-32998 at the same time. Cheers.

          Show
          Jérôme Mouneyrac added a comment - Also need to integrate MDL-32998 at the same time. Cheers.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just for reference we have agreed to:

          • For SOAP:
          • Return the error code in the faultactor and also keep it at the end of the message. So client developers can decide from where to fetch the error code.
          • Return the debuginfo into faultdetails.
          • For XMLRPC:
          • Return the error code at the end of the message
          • Also return the numerical representation of the errorcode as faultcode so, once again, client developers can decide which one to use.
          • For REST:
          • No change needed, nor in json nor xml modes. It's free-format and each piece of information has its own attribute/tag.

          Important note: Each time we update the Zend/Soap implementation we'll need to copy the SoapServer::handle() method to our moodle_zend_soap_server, until they fix the handling of faultactor and faultdetails.

          Show
          Eloy Lafuente (stronk7) added a comment - Just for reference we have agreed to: For SOAP: Return the error code in the faultactor and also keep it at the end of the message. So client developers can decide from where to fetch the error code. Return the debuginfo into faultdetails. For XMLRPC: Return the error code at the end of the message Also return the numerical representation of the errorcode as faultcode so, once again, client developers can decide which one to use. For REST: No change needed, nor in json nor xml modes. It's free-format and each piece of information has its own attribute/tag. Important note: Each time we update the Zend/Soap implementation we'll need to copy the SoapServer::handle() method to our moodle_zend_soap_server, until they fix the handling of faultactor and faultdetails.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          2 things to do:

          1) Amend a bit the code to fix some comments. I'll do that now.
          2) Document in http://docs.moodle.org/dev/Errors_handling_in_web_services how each piece is returned for each supported protocol. All yours, Jerome.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - 2 things to do: 1) Amend a bit the code to fix some comments. I'll do that now. 2) Document in http://docs.moodle.org/dev/Errors_handling_in_web_services how each piece is returned for each supported protocol. All yours, Jerome. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Jérôme Mouneyrac added a comment -

          Eloy I completed http://docs.moodle.org/dev/Errors_handling_in_web_services and discovered that it would be better for the errorcode to be => component/errorcode so we avoid conflict when the errorcode is the same in two component. When no module exists then it's 'error'.

          If you agree I can do the change.

          Show
          Jérôme Mouneyrac added a comment - Eloy I completed http://docs.moodle.org/dev/Errors_handling_in_web_services and discovered that it would be better for the errorcode to be => component/errorcode so we avoid conflict when the errorcode is the same in two component. When no module exists then it's 'error'. If you agree I can do the change.
          Hide
          Jérôme Mouneyrac added a comment -

          Ok I made the changes Eloy, chery-pick if you like it: https://github.com/mouneyrac/moodle/commit/59e9fa5b873a0b220a242de3dbf8a90fce49201f

          Show
          Jérôme Mouneyrac added a comment - Ok I made the changes Eloy, chery-pick if you like it: https://github.com/mouneyrac/moodle/commit/59e9fa5b873a0b220a242de3dbf8a90fce49201f
          Hide
          Adrian Greeve added a comment -

          Errors returned match the expected descriptions mentioned in the development documents.

          Thanks Jerome for all of your help!

          Show
          Adrian Greeve added a comment - Errors returned match the expected descriptions mentioned in the development documents. Thanks Jerome for all of your help!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks for the hard work, closing this as fixed.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: