Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32949

Web service exception should return errorcode (from moodle exception)

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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:

      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);
          }

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            stronk7 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
            stronk7 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
            jerome 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
            jerome 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
            jerome Jérôme Mouneyrac added a comment -

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

            Show
            jerome Jérôme Mouneyrac added a comment - Also need to integrate MDL-32998 at the same time. Cheers.
            Hide
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
            Hide
            jerome 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
            jerome 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
            jerome 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
            jerome 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
            abgreeve Adrian Greeve added a comment -

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

            Thanks Jerome for all of your help!

            Show
            abgreeve Adrian Greeve added a comment - Errors returned match the expected descriptions mentioned in the development documents. Thanks Jerome for all of your help!
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  25/Jun/12