Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Minor
    • Resolution: Won't Fix
    • Affects Version/s: 1.1.0-dp1
    • Fix Version/s: None
    • Component/s: library
    • Security Level: Public
    • Labels:
      None
    • Environment:
      Every

      Description

      Instead of showing warnings on connect, throw exceptions. This makes errors Better to handle up the call-stack.

      I've already done some coding, tests need to be adapted then. See the latest commits:
      https://github.com/daschl/php-ext-couchbase/commits/

      If you want to integrate it I can do the rest of the workload and then submit a pull request.
      What do you think?

      Regards,
      michael

        Issue Links

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

          Hide
          jan Jan Lehnardt (Inactive) added a comment -

          Heh, okay.

          I saw on twitter that you started a patch, would you be interested in going through the whole code and adding exception calls?

          I'd probably encapsulate this in a new function:

          php_couchbase_error_or_exception(zend_bool is_exception, int level, char *message TSRLMS_CC)
          {
          if(is_exception)

          { zend_throw_exception(...) }

          else

          { php_error_docref(...) }

          }

          where is_exception is primed with the value of ::throwException from above. Does that make sense? Of course there is a bit of handwaving here

          Show
          jan Jan Lehnardt (Inactive) added a comment - Heh, okay. I saw on twitter that you started a patch, would you be interested in going through the whole code and adding exception calls? I'd probably encapsulate this in a new function: php_couchbase_error_or_exception(zend_bool is_exception, int level, char *message TSRLMS_CC) { if(is_exception) { zend_throw_exception(...) } else { php_error_docref(...) } } where is_exception is primed with the value of ::throwException from above. Does that make sense? Of course there is a bit of handwaving here
          Hide
          daschl Michael Nitschinger added a comment -

          Sounds good, that was I was thinking about. Actually I had the tests done already too

          I'll make a branch on my first and then ping you when I got it done, right?

          Show
          daschl Michael Nitschinger added a comment - Sounds good, that was I was thinking about. Actually I had the tests done already too I'll make a branch on my first and then ping you when I got it done, right?
          Hide
          jan Jan Lehnardt (Inactive) added a comment -

          sure, feel free to CC me on github on the progress you make, maybe I get bored and pitch in

          Show
          jan Jan Lehnardt (Inactive) added a comment - sure, feel free to CC me on github on the progress you make, maybe I get bored and pitch in
          Hide
          ingenthr Matt Ingenthron added a comment -

          Mark: Please determine if this issue is still valid and help me triage it into the appropriate release. I've assigned it to you for triage, not necessarily to fix it.

          Show
          ingenthr Matt Ingenthron added a comment - Mark: Please determine if this issue is still valid and help me triage it into the appropriate release. I've assigned it to you for triage, not necessarily to fix it.
          Hide
          mnunberg Mark Nunberg added a comment -

          Connection error handling has been discussed in later issues. Exceptions in general may be added in subsequent releases (and is a rather requested feature).

          Marking this as WONTFIX because this is a later task for a completely new release/API

          Show
          mnunberg Mark Nunberg added a comment - Connection error handling has been discussed in later issues. Exceptions in general may be added in subsequent releases (and is a rather requested feature). Marking this as WONTFIX because this is a later task for a completely new release/API

            People

            • Assignee:
              mnunberg Mark Nunberg
              Reporter:
              daschl Michael Nitschinger
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 2h
                2h
                Remaining:
                Remaining Estimate - 2h
                2h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Gerrit Reviews

                  There are no open Gerrit changes