Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Allow to configure a default type map #50

Open
2 tasks done
acelaya opened this issue Nov 24, 2019 · 6 comments
Open
2 tasks done

Allow to configure a default type map #50

acelaya opened this issue Nov 24, 2019 · 6 comments

Comments

@acelaya
Copy link
Contributor

acelaya commented Nov 24, 2019

Currently, when the ProblemDetailsResponseFactory::createResponse method is called without a type, it tries to infer it from the status, generating values like https://httpstatus.es/404 or https://httpstatus.es/500.

This mainly happens when the ProblemDetailsNotFoundHandler is hit, or the ProblemDetailsMiddleware catches an exception not implementing ProblemDetailsExceptionInterface.

If your API has a specific pattern when generating the types for other errors, it's a bit inconsistent that these two errors follow a different pattern.

It would be nice to be able to define a config map where you set the default type to be used for "every" status code when a value for type was not provided. Something like this:

<?php

return [

    'zend_problem_details' => [
        'default_type_fallbacks' => [
            404 => '/my-app/errors/resource-not-found',
            500 => '/my-app/errors/unknown',
        ],
    ],

];

Then, by injecting this map in the ProblemDetailsResponseFactory, the createTypeFromStatus method could be refactored to look like this:

private function createTypeFromStatus(int $status) : string
{
    return $this->defaultTypeFallbacks[$status] ?? sprintf('https://httpstatus.es/%s', $status);
}

So it would continue working the same when the config map is not defined, but every user would have control over the values generated for the type.

I'm open to implement this if you think it's useful.

@geerteltink
Copy link
Member

geerteltink commented Nov 24, 2019

I'm not sure if it is useful (although others might think it is). There is already a way to create custom exceptions.

namespace Api;

use DomainException as PhpDomainException;
use Zend\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait;
use Zend\ProblemDetails\Exception\ProblemDetailsExceptionInterface;

class ResourceNotFoundException extends PhpDomainException implements ProblemDetailsExceptionInterface
{
    use CommonProblemDetailsExceptionTrait;

    public static function create(string $message, array $details) : self
    {
        $e = new self($message)
        $e->status = 404;
        $e->detail = $message;
        $e->type = 'https://example.com/my-app/errors/resource-not-found';
        $e->title = 'Resource not found';
        $e->additional['transaction'] = $details;
        return $e;
    }
}

You throw a ResourceNotFoundException('The user resource could not be found') and all other properties are set with your custom defaults.

@acelaya
Copy link
Contributor Author

acelaya commented Nov 24, 2019

Yes. In any case in which I manually throw an exception, I'm in control of everything which is going to be returned.

The problem is when something I cannot control happens.

For example, when a request is performed to a route which does not match any configured route.

Or if for some reason, some third party exception ends up not being caught and goes up to the error handling middleware (I try to catch those and wrap them into domain exceptions which implement problem details, but something could go wrong).

I would like to be able to have a consistent set of API errors, even for those cases.

However, I have already implemented a workaround (not so nice, but it works), so if you really think this is not useful, I will live with it :)

@geerteltink
Copy link
Member

Thanks for the explanation. If you want to create a PR, that would be awesome.

@acelaya
Copy link
Contributor Author

acelaya commented Nov 25, 2019

Great! I will sure do :)

@acelaya
Copy link
Contributor Author

acelaya commented Nov 26, 2019

Just created the PR: #51

Monitoring the builds to make sure they pass.

@weierophinney
Copy link
Member

This repository has been closed and moved to mezzio/mezzio-problem-details; a new issue has been opened at mezzio/mezzio-problem-details#1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants