[1.x] Add activeOrFail method

[martinbean]

Adds a new activeOrFail (taking the naming convention from Eloquent models) method to check if a feature is active, throwing an exception if it isn’t.

The motivation for this method is to reduce the need of conditionals when feature-flagging code paths. For example, a queued job that can only be queued if a specific feature is active. Before, I’d have something like this:

public function __construct(User $user)
{
    if (Feature::inactive('feature-v2')) {
        throw new RuntimeException('Feature [feature-v2] is not active.');
    }

    $this->user = $user;
}

With the new method, this can now be simplified to:

  public function __construct(User $user)
  {
-     if (Feature::inactive('feature-v2')) {
-         throw new RuntimeException('Feature [feature-v2] is not active.');
-     }
+     Feature::activeOrFail('feature-v2');

      $this->user = $user;
  }

[timacdonald]

My concern here is what the *fail() APIs communicate across our ecosystem.

As far as I'm aware, all the *fail() or abort() methods are used as a kind of circuit breaker that are then handled elsewhere, i.e., firstOrFail throws an exception that the handler captures and converts into a 404 while the exception is not logged or reported to any error handler. abort calls result in HTTP responses but are not reported to exception trackers or logs.

This method would throw an exception, result in a 500, report to exception trackers, and be logged.

Also, the *fail naming, to me, communicates a 404. That does not feel like a suitable response code here, though.

Feature::activeOrAbort($feature, $code, ) is probably more applicable method name here, but we don't have any precedence for that in the framework.

[martinbean]

@timacdonald I didn’t really consider HTTP statuses, as features can be used in contexts other than HTTP. Particularly the example I gave being a queued job.

My reckoning was, for the exception to indeed be logged in exception trackers, as developers would then know they have code paths trying to call flagged features, either unintentionally or inadvertently. For example, I’m working on a huge refactor of a core feature, so want to know where any v1 methods are getting called if v2 is active, and vice versa.

Happy to adjust naming to suit any existing conventions or standards, but believe a “blow up if this feature is inactive” method still has merit 😄 I can’t be the only developer wrapping feature checks in an if statement just to throw an error.

[taylorotwell]

@martinbean curious how you would be using this? You actually do throw 500s to users if a feature isn't active?

[martinbean]

@taylorotwell I just thought a convenience method to throw some exception if a feature isn’t active would be useful, so I can catch any instances of trying to call flagged features where they shouldn’t be called, and remove a load of if statements from code.

The implementation in my PR defined a dedicated exception class (FeatureInactiveException) so developers could define their own logic. I didn’t want to dictate a particular status code as I know that would then just open a whole other argument conversation on which HTTP status code is the most appropriate one. Developers can then choose for themselves what status code a missing or inactive feature should return in a HTTP context.

[taylorotwell]

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!