3

I'm in the process of building a routing system for learning purposes and have encountered an issue which I think is a bit in the grey area of best practices. Can you guys help me decide if this is wrong, okay or could be done in a different way.

When resolving an incoming request and a route resource has been matched the HTTP request method is validated against the route's supported HTTP methods to ensure its capable of handling such requests. If this is not the case an exception of type UnsupportedMethodException is thrown.

The HTTP status code documentation states that if a request method is denied/unsupported an Allow header containing a list of supported methods should be returned in the response.

10.4.6 405 Method Not Allowed

The method specified in the Request-Line is not allowed for the resource identified by the Request-URI. The response MUST include an Allow header containing a list of valid methods for the requested resource.

So to accommodate this I thought of extending the usage of the otherwise generic exception to require a sequential array of supported methods in its constructor.

class UnsupportedMethodException extends \RuntimeException {

    /**
     * A sequential array of supported HTTP request methods.
     *
     * @var array $supported
     */
    private $supported = [];

    /**
     * @param string     $message           The typical exception message.
     * @param array      $supportedMethods  A sequential array of HTTP request methods the matched route supports.
     * @param \Exception $previous          Any previous exceptions
     */
    public function __construct($message, array $supportedMethods, \Exception $previous = null)
    {
        parent::__construct($message, null, $previous);
        $this->supported = $supportedMethods;
    }

    /**
     * @return array
     */
    public function getSupportedMethods()
    {
        return $this->supported;
    }

}

With the usage of type hinting the catch block provides I now have a solid method for fetching the supported methods and generating a proper HTTP response.

So my question is, do you guys think this is a good practice? Something tells me the exception class shouldn't contain such logic, but I have no proof/reference to back this up.

Improve this question

3 Answers 3

Reset to default
3

Storing information related to the exceptional circumstance in the exception is definitely a good thing, as it can allow a handler to fix & retry or fail gracefully. It's not logic but data, so you don't have to worry about the exception class being too complex. If the exception took a route and extracted the supported methods, that would be getting too coupled, but taking the methods as an argument is just dumb enough.

Other questions, "How to write a good exception message" and "Why do many exception messages not contain useful details?", are closely related but focused on the message, rather than other exception fields.

Improve this answer
1
  • Thanks for your answer. I understand your that its data and logic which is stored inside the class. I think what I hinted at was the logic the followed it the exception was thrown. Would it seem odd to you that an exception provided the data?
    – AnotherGuy
    Commented Apr 20, 2015 at 9:14
1

If your exception did in fact contain any logic, I would take issue with it, but it does not, so I think it is fine. What it contains is the range (the set, actually) of accepted values, which is not a bad idea. (It might be unnecessary, because the range of accepted values is fixed and known in advance, but not bad at all.)

I think that it should also contain the name of the method that was not allowed, and possibly the URI that failed, since the "Method Not Allowed" response pertains to specific URIs. This information might not be useful for constructing the http response, but it should be useful for logging.

Improve this answer
1
  • Thank you for your answer. I very much like your idea of providing the invalid method along with the URI for logging. Thanks!
    – AnotherGuy
    Commented Apr 20, 2015 at 16:16
0

Using an exception to be part of program-flow is an anti-pattern, they should not be used for things that you expect to happen - hence the name!

I'd say that practicality trumps such rules however, but here I'd question why you are throwing such an exception and simply not returning the 405 result code - if the method succeeded you wouldn't throw an exception to return a 200 code, so I don't see throwing this exception is best practice here.

But, if you are throwing this, then I would call a function in the returning handler that fetches the allowed methods, not pass the data back via the exception.

Improve this answer
4
  • Because a route matcher does not need to know about responses. It just knows that the route failed to match and want to tell whomever it may concern why it failed.
    – Cerad
    Commented Apr 20, 2015 at 12:32
  • @Cerad so how does it know what all the valid methods are which is what the OP is intending to put in the exception object.
    – gbjbaanb
    Commented Apr 20, 2015 at 12:41
  • It's a http route matcher. Of course it knows which http methods it's allowed to match against.
    – Cerad
    Commented Apr 20, 2015 at 13:15
  • Thanks for your answer. I understand your reasoning behind the usage of exceptions. The current architecture is so that a single URI can respond to different HTTP methods, so the request matching routine has access to all accepted methods to avoid instantiating resources which we know beforehand are invalid. I throw an exception as I assume only (the one building the site) should issue request other than GET. If a post/put... is issued I would argue its from a link I created.
    – AnotherGuy
    Commented Apr 20, 2015 at 16:15

Not the answer you're looking for? Browse other questions tagged or ask your own question.