Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.2
    • Component/s: Web Services
    • Labels:
      None

      Description

      When an access control exception is raised by web services, add a link to a Moodledocs explaining the common reasons causing the problem.

        Gliffy Diagrams

          Attachments

            Activity

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

            I know that during the integration process we don't like to change any strings into STABLE versions. However it would be so much nicer for debugging any web service related problem in previous version. I give my +10 to backport this issue on 2.0 and 2.1. Let me know if it's ok, in this case I'll create some branches for you to pull and integrate.
            Cheers,
            Jerome

            Show
            jerome Jérôme Mouneyrac added a comment - - edited I know that during the integration process we don't like to change any strings into STABLE versions. However it would be so much nicer for debugging any web service related problem in previous version. I give my +10 to backport this issue on 2.0 and 2.1. Let me know if it's ok, in this case I'll create some branches for you to pull and integrate. Cheers, Jerome
            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
            Hide
            skodak Petr Skoda added a comment -

            I do not like this hack, error message for users should be general, developers need more information (they get it from debug messages).

            In any case exceptions should be "exceptional" you should not rely on them, for example when doing standard web interface the links we create should not lead to dead pages with exceptions. Here the client WS application should not call webservices with random parameters, instead use WS query first to find out what you can access on server and then make query with valid parameters. The exceptions shoudl happen when somebody concurrently modifies moodle data - such as delete course, then of course users may get cryptic messages, but the application has to recover and it must stop using old parameters...

            It might be beneficial to add a new field to the user table "debug" which would selectively enable debugging for specific users, or a CFG->debugusers which enables max debugging for userids in it. We have discusses this several times before, maybe it is time to finally do something about this...

            my -1 because this is not a proper solution imo

            Show
            skodak Petr Skoda added a comment - I do not like this hack, error message for users should be general, developers need more information (they get it from debug messages). In any case exceptions should be "exceptional" you should not rely on them, for example when doing standard web interface the links we create should not lead to dead pages with exceptions. Here the client WS application should not call webservices with random parameters, instead use WS query first to find out what you can access on server and then make query with valid parameters. The exceptions shoudl happen when somebody concurrently modifies moodle data - such as delete course, then of course users may get cryptic messages, but the application has to recover and it must stop using old parameters... It might be beneficial to add a new field to the user table "debug" which would selectively enable debugging for specific users, or a CFG->debugusers which enables max debugging for userids in it. We have discusses this several times before, maybe it is time to finally do something about this... my -1 because this is not a proper solution imo
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Jerome - reopening as discussed.
            I agree with Petr's comments here as well.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Jerome - reopening as discussed. I agree with Petr's comments here as well. Cheers Sam
            Hide
            jerome Jérôme Mouneyrac added a comment -

            What about a $CFG->debugws and moodle_exception() always add the $debuginfo when it's set to true? It seems a reasonable feature.

            Show
            jerome Jérôme Mouneyrac added a comment - What about a $CFG->debugws and moodle_exception() always add the $debuginfo when it's set to true? It seems a reasonable feature.
            Hide
            jerome Jérôme Mouneyrac added a comment -

            hmm actually maybe it's just the Zend server the issue...

            Show
            jerome Jérôme Mouneyrac added a comment - hmm actually maybe it's just the Zend server the issue...
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I created a Zend server related issue: http://tracker.moodle.org/browse/MDL-29435

            Submitting a small change of the debuginfo.

            Show
            jerome Jérôme Mouneyrac added a comment - I created a Zend server related issue: http://tracker.moodle.org/browse/MDL-29435 Submitting a small change of the debuginfo.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The change looks ok for me, but I have not followed this issue evolution, so Sam, if you can take a look to it... TIA!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The change looks ok for me, but I have not followed this issue evolution, so Sam, if you can take a look to it... TIA!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Looks good to me as well Jerome, however you've forgotten to include the MDL issue number in the commit message.
            Are you happy for me to cherry-pick your changes and amend the commit message to contain the issue number or would you rather fix it yourself?

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Looks good to me as well Jerome, however you've forgotten to include the MDL issue number in the commit message. Are you happy for me to cherry-pick your changes and amend the commit message to contain the issue number or would you rather fix it yourself? Cheers Sam
            Hide
            jerome Jérôme Mouneyrac added a comment -

            I'll fix it

            Show
            jerome Jérôme Mouneyrac added a comment - I'll fix it
            Hide
            jerome Jérôme Mouneyrac added a comment -

            done

            Show
            jerome Jérôme Mouneyrac added a comment - done
            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.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Tested during integration

            Show
            samhemelryk Sam Hemelryk added a comment - Tested during integration
            Hide
            samhemelryk Sam Hemelryk added a comment -

            ...and passed

            Show
            samhemelryk Sam Hemelryk added a comment - ...and passed
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            And this code has been spread to all Moodle git and cvs repositories. Many thanks! Closing.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - And this code has been spread to all Moodle git and cvs repositories. Many thanks! Closing. Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  5/Dec/11