Announcement

Collapse
No announcement yet.

API Error Handling with Invalid Attributes

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

  • API Error Handling with Invalid Attributes

    If I pass in a "where" filter for an attribute that does not exist (for example, a misspelling like "nmae" instead of "name") the API seems to return rows as if I passed no filters in. IMO the API should throw an error if you try to filter on a non-existent attribute. Returning rows as if there was no filter is risky as well, as if the misspelling/bug goes uncaught your code could be operating on entities it's not supposed to.


  • #2
    It's a controversial point. Will require a lot of testing. Since sometimes there could be passed not existing seach params handled separately for a not standard behaviour.
    Last edited by yuri; 01-19-2018, 08:24 AM.

    Comment


    • #3
      I can't imagine a use case where people would run a search against fields that don't exist. Maybe I am misunderstanding though, as you are infinitely more familiar with this product than I am.

      Comment


      • #4
        Kurt I agree with you. An invalid search parameter should either return zero results or an error. I can't think of a use case where it would be useful to see results when the query was invalid in the first place.

        Comment


        • #5
          It's already used. I think we can live with this.

          Returning an empty list is a bad idea. Throwing an error would be more proper.
          Last edited by yuri; 01-22-2018, 07:48 AM.

          Comment


          • #6
            yuri Why is returning an empty list bad? The client should be checking the length of the array before trying to process it. It's very unusual to return results from an invalid query. An empty array makes the most sense for that use case because the error status is only supposed to be used when the call had an error, not the results. An empty array isn't an error, but a bad field should be an error.

            This is happening with internal objects as well and it's very confusing. For example,

            PHP Code:

            75    $approver 
            $this->getEntityManager()->getRepository('User')->where(array(
            76       'initials' => $approvedBy
            77     
            ))->findOne();
            78
            79     
            if(!$approver) {
            80       fwrite($hw,"Incorrect number of approvers (initials = '$approvedBy')\n");
            81       return false;
            82     
            This doesn't work as intended because
            PHP Code:
            $approver 
            is set to a totally unexpected user due to the fact that
            PHP Code:
            $approvedBy 
            is null in this case. The first result returned is pretty much random. I spent hours trying to figure out what was wrong before I realized the repository class is trying to return something even if no results are found. The result in this case must be empty or null for it to make any sense because every user has non-null initials

            In a similar situation, I used 'where initial => $approvedBy' instead of 'initials' and the result was not empty. That should have thrown an exception because 'initial' isn't a field in my entity. Instead, I got a random row as a result. To fix this, you have to inspect each result to see if it makes any sense, which defeats the whole purpose of the where query. I should be able to rely on my database calls to return the data I asked for instead of double checking all of the results.

            I ended up needing to do this to every call:

            PHP Code:

            84     
            if($approvedBy != $approver->get('initials')) {
            85       fwrite($hw,"Wrong approver returned: ");
            86       return false;
            87     
            I should never get a result that fails that check because the where clause is supposed to make it impossible, but it happens often.
            Last edited by bandtank; 02-17-2018, 11:00 PM.

            Comment


            • #7
              It needs a lot of testing.

              You can try to patch it for your instance:

              https://github.com/espocrm/espocrm/b.../Base.php#L776

              PHP Code:
              if (!isset($entity->fields[$field])) {
                  
              $whereParts[] = '0'// add this line
                  
              continue;

              Comment


              • #8
                This fix brought about many issues in many places.

                Comment

                Working...
                X