Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APIv4 - Fix HTTP status code selection #19533

Merged
merged 1 commit into from
Feb 4, 2021

Conversation

totten
Copy link
Member

@totten totten commented Feb 4, 2021

This is a follow-up to #19526 which addresses a typo that causes a misbehavior in reporting the HTTP status code.

Before

If an API request encounters an exception, then it always returns HTTP 403.

After

If an API request encounters an exception, then:

  • It may return HTTP 403 (for an authorization exception)
  • It may return HTTP 500 (for any other/unrecognized exception)

Comments

To test this, I just hacked an API locally and then ran it via curl or Firefox:

diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php
index 5689252494..8aee88f04e 100644
--- a/Civi/Api4/Entity.php
+++ b/Civi/Api4/Entity.php
@@ -33,6 +33,8 @@ class Entity extends Generic\AbstractEntity {
    * @return Action\Entity\Get
    */
   public static function get($checkPermissions = TRUE) {
+//A//    throw new \Exception('asdfasdf');
+//B//    throw new \Civi\API\Exception\UnauthorizedException('fdsa');
     return (new Action\Entity\Get('Entity', __FUNCTION__))
       ->setCheckPermissions($checkPermissions);
   }

This is a follow-up to civicrm#19526 which addresses a typo that causes a misbehavior in reporting the HTTP status code.

Before
------

If an API request encounters an exception, then it always returns HTTP 403.

After
-----

If an API request encounters an exception, then:

* It may return HTTP 403 (for an authorization exception)
* It may return HTTP 500 (for any other/unrecognized exception)
@civibot
Copy link

civibot bot commented Feb 4, 2021

(Standard links)

@colemanw
Copy link
Member

colemanw commented Feb 4, 2021

Looks good.

@seamuslee001 seamuslee001 merged commit 4e68662 into civicrm:master Feb 4, 2021
@totten totten deleted the master-api4-status-code branch February 5, 2021 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants