Details

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

      Description

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

        Activity

        Hide
        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
        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
        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
        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
        Petr Škoda 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
        Petr Škoda 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
        Sam Hemelryk added a comment -

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

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Jerome - reopening as discussed. I agree with Petr's comments here as well. Cheers Sam
        Hide
        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
        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
        Jérôme Mouneyrac added a comment -

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

        Show
        Jérôme Mouneyrac added a comment - hmm actually maybe it's just the Zend server the issue...
        Hide
        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
        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
        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
        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
        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
        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
        Jérôme Mouneyrac added a comment -

        I'll fix it

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

        done

        Show
        Jérôme Mouneyrac added a comment - done
        Hide
        Sam Hemelryk added a comment -

        Thanks Jerome this has been integrated now.

        Show
        Sam Hemelryk added a comment - Thanks Jerome this has been integrated now.
        Hide
        Sam Hemelryk added a comment -

        Tested during integration

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

        ...and passed

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

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

        Ciao

        Show
        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: