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

          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