Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2.2
    • Component/s: Web Services
    • Labels:

      Description

      Some Fabio's comments originally in MDL-30878 that concerns a Coding error exception with core_role_assign_roles

      About the return value, returning null actually causes an execution error (at least on my installations).

      I took this example from the forum, but the error is the same.

      <EXCEPTION class="coding_exception">
      <MESSAGE>Coding error detected, it must be fixed by a programmer:
      PHP catchable fatal error</MESSAGE>
      <DEBUGINFO>Argument 1 passed to external_api::clean_returnvalue()
      must be an instance of external_description, null given, called in
      /var/www/moodle/moodle/webservice/rest/locallib.php on line 88 and
      defined</DEBUGINFO>
      </EXCEPTION>

      What happens is that on the webservice definition, the update_users_returns() is returning null, but a structure of the type external_description must be returned always (or at least, that is what I understand).
      This is why I've returned the id's instead of null.
      I would like to know what is your opinion about this. Another user has complained already, because a lot of webservice functions are declaring their return values as null, which raises these errors and break the webservices output, even though they do what they are supposed to do. This forum discussion is an example: http://moodle.org/mod/forum/discuss.php?d=193402

      I have seen that this behavior differs on the 2.1 implementation.
      This is why it works on 2.1 but not on 2.2: the response generator is different on each version, and on 2.2 version it does not accept null as a return value.

      So it actually seems more like a bug on the response generator then on the webservice definitions, as I thought it was.

        Gliffy Diagrams

          Issue Links

            Activity

            jerome Jérôme Mouneyrac created issue -
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Hi Fabio, about the second quote, it did work in 2.1 because the REST server didn't validate the return value against the description. But it is most likely that the SOAP and XMLRPC servers failed in 2.1. Anyway good catch

            Show
            jerome Jérôme Mouneyrac added a comment - Hi Fabio, about the second quote, it did work in 2.1 because the REST server didn't validate the return value against the description. But it is most likely that the SOAP and XMLRPC servers failed in 2.1. Anyway good catch
            salvetore Michael de Raadt made changes -
            Field Original Value New Value
            Fix Version/s STABLE backlog [ 10463 ]
            Description Some Fabio's comments originally in MDL-30878 that concerns a Coding error exception with core_role_assign_roles
            {quote}
            About the return value, returning null actually causes an execution error (at least on my installations).

            I took this example from the forum, but the error is the same.

            <EXCEPTION class="coding_exception">
            <MESSAGE>Coding error detected, it must be fixed by a programmer:
            PHP catchable fatal error</MESSAGE>
            <DEBUGINFO>Argument 1 passed to external_api::clean_returnvalue()
            must be an instance of external_description, null given, called in
            /var/www/moodle/moodle/webservice/rest/locallib.php on line 88 and
            defined</DEBUGINFO>
            </EXCEPTION>

            What happens is that on the webservice definition, the update_users_returns() is returning null, but a structure of the type external_description must be returned always (or at least, that is what I understand).
            This is why I've returned the id's instead of null.
            I would like to know what is your opinion about this. Another user has complained already, because a lot of webservice functions are declaring their return values as null, which raises these errors and break the webservices output, even though they do what they are supposed to do. This forum discussion is an example: http://moodle.org/mod/forum/discuss.php?d=193402
            {quote}
            {quote}
            I have seen that this behavior differs on the 2.1 implementation.
            This is why it works on 2.1 but not on 2.2: the response generator is different on each version, and on 2.2 version it does not accept null as a return value.

            So it actually seems more like a bug on the response generator then on the webservice definitions, as I thought it was.
            {quote}
            Some Fabio's comments originally in MDL-30878 that concerns a Coding error exception with core_role_assign_roles
            {quote}
            About the return value, returning null actually causes an execution error (at least on my installations).

            I took this example from the forum, but the error is the same.

            <EXCEPTION class="coding_exception">
            <MESSAGE>Coding error detected, it must be fixed by a programmer:
            PHP catchable fatal error</MESSAGE>
            <DEBUGINFO>Argument 1 passed to external_api::clean_returnvalue()
            must be an instance of external_description, null given, called in
            /var/www/moodle/moodle/webservice/rest/locallib.php on line 88 and
            defined</DEBUGINFO>
            </EXCEPTION>

            What happens is that on the webservice definition, the update_users_returns() is returning null, but a structure of the type external_description must be returned always (or at least, that is what I understand).
            This is why I've returned the id's instead of null.
            I would like to know what is your opinion about this. Another user has complained already, because a lot of webservice functions are declaring their return values as null, which raises these errors and break the webservices output, even though they do what they are supposed to do. This forum discussion is an example: http://moodle.org/mod/forum/discuss.php?d=193402
            {quote}

            {quote}
            I have seen that this behavior differs on the 2.1 implementation.
            This is why it works on 2.1 but not on 2.2: the response generator is different on each version, and on 2.2 version it does not accept null as a return value.

            So it actually seems more like a bug on the response generator then on the webservice definitions, as I thought it was.
            {quote}
            Labels triaged
            salvetore Michael de Raadt made changes -
            Link This issue is duplicated by MDL-31082 [ MDL-31082 ]
            Hide
            salvetore Michael de Raadt added a comment -

            Some additional information is presented in the linked duplicate issue.

            Show
            salvetore Michael de Raadt added a comment - Some additional information is presented in the linked duplicate issue.
            jerome Jérôme Mouneyrac made changes -
            Priority Minor [ 4 ] Critical [ 2 ]
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I found the issue, it impacts all REST call returning a description == null. Fixing it as it was done for the Zend servers...

            Show
            jerome Jérôme Mouneyrac added a comment - I found the issue, it impacts all REST call returning a description == null. Fixing it as it was done for the Zend servers...
            jerome Jérôme Mouneyrac made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            jerome Jérôme Mouneyrac made changes -
            Pull Master Diff URL https://github.com/mouneyrac/moodle/compare/master...MDL-31077
            Pull Master Branch MDL-31077
            Priority Critical [ 2 ] Blocker [ 1 ]
            Pull from Repository git://github.com/mouneyrac/moodle.git
            jerome Jérôme Mouneyrac made changes -
            Comment [ Use the REST client () and call core_role_assign_roles. null should be returned (no coding error displayed). ]
            jerome Jérôme Mouneyrac made changes -
            Testing Instructions Use the REST client (https://github.com/moodlehq/sample-ws-clients/tree/master/PHP-REST) to call core_role_assign_roles. 'null' should be returned, you should not see any coding error.
            jerome Jérôme Mouneyrac made changes -
            Affects Version/s 2.1.4 [ 11452 ]
            jerome Jérôme Mouneyrac made changes -
            jerome Jérôme Mouneyrac made changes -
            Summary core_role_assign_roles ws function throw a Coding error exception REST server fails to return null
            jerome Jérôme Mouneyrac made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            nebgor Aparup Banerjee made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Peer reviewer nebgor
            Hide
            nebgor Aparup Banerjee added a comment -

            Jerome, i've had a look at the code and its parfait.

            (although i'm not sure if using null has any impact on ws/rest)

            Show
            nebgor Aparup Banerjee added a comment - Jerome, i've had a look at the code and its parfait. (although i'm not sure if using null has any impact on ws/rest)
            nebgor Aparup Banerjee made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Hide
            jerome Jérôme Mouneyrac added a comment -

            Thanks Apu. It is fixed the exact same way than the Zend servers. I tested on 2.2 and HEAD (the issue doesn't exist in 2.1 has the return values validation has just been added to 2.2), all works fine.

            Show
            jerome Jérôme Mouneyrac added a comment - Thanks Apu. It is fixed the exact same way than the Zend servers. I tested on 2.2 and HEAD (the issue doesn't exist in 2.1 has the return values validation has just been added to 2.2), all works fine.
            jerome Jérôme Mouneyrac made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Currently in integration Yes [ 10041 ]
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jerome - this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jerome - this has been integrated now.
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Fix Version/s 2.2.2 [ 11552 ]
            Fix Version/s STABLE backlog [ 10463 ]
            gerry Gerard Caulfield made changes -
            Tester gerry
            gerry Gerard Caulfield made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            gerry Gerard Caulfield added a comment -

            Before patch:

            <?xml version="1.0" encoding="UTF-8" ?>
            <EXCEPTION class="coding_exception">
            <MESSAGE>Coding error detected, it must be fixed by a programmer: PHP catchable fatal error</MESSAGE>
            <DEBUGINFO>Argument 1 passed to external_api::clean_returnvalue() must be an instance of external_description, null given, called in /home/moodleuser/src/moodle/pro/22/webservice/rest/locallib.php on line 88 and defined</DEBUGINFO>
            </EXCEPTION>

            After patch:

            <?xml version="1.0" encoding="UTF-8" ?>
            <RESPONSE>
            </RESPONSE>

            Test passed! =)

            Show
            gerry Gerard Caulfield added a comment - Before patch: <?xml version="1.0" encoding="UTF-8" ?> <EXCEPTION class="coding_exception"> <MESSAGE>Coding error detected, it must be fixed by a programmer: PHP catchable fatal error</MESSAGE> <DEBUGINFO>Argument 1 passed to external_api::clean_returnvalue() must be an instance of external_description, null given, called in /home/moodleuser/src/moodle/pro/22/webservice/rest/locallib.php on line 88 and defined</DEBUGINFO> </EXCEPTION> After patch: <?xml version="1.0" encoding="UTF-8" ?> <RESPONSE> </RESPONSE> Test passed! =)
            gerry Gerard Caulfield made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many.

            Nah, joking, many thanks! Closing this a fixed, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Your nice code represents only 1/46 of the issues that have been sent upstream this week, so thanks, but not many. Nah, joking, many thanks! Closing this a fixed, ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 27/Jan/12
            jerome Jérôme Mouneyrac made changes -
            Link This issue is duplicated by MDL-31538 [ MDL-31538 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12