Skip to content
This repository has been archived by the owner on Feb 10, 2023. It is now read-only.

Error codes #25

Open
sagikazarmark opened this issue Apr 30, 2016 · 4 comments
Open

Error codes #25

sagikazarmark opened this issue Apr 30, 2016 · 4 comments
Milestone

Comments

@sagikazarmark
Copy link

What's the idea behind using exception messages like 'CMP-001: Something bad happpened'?

I always found it a little bit hard to follow, and I remember we even had issues with conflicting codes. If the point is the representation, we could make the exception handler put the code before the message. Something like:

interface FuelException
{
    public function getComponentName();
}

class ComponentLoad extends \RuntimeException implements FuelException
{
    const CLASS_NOT_FOUND = 1;

    public function getComponentName()
    {
        return 'FOU';
    }
}

Then use it like:

throw new ComponentLoad("Unable to load [$fullName]: Class not found", ComponentLoad::CLASS_NOT_FOUND);

This way we would use the builtin exception code, would track the codes at one place (Exception class), would make changes easier to follow (constants) and would be possible to format the message in a custom exception handler.

@WanWizard
Copy link

WanWizard commented May 1, 2016

The idea was that messages in the code are in the first place descriptive for ourselfs, and not end-user messages. The code "CMP-001" makes it easy to find the error in the code since it makes similar messages unique. With lots of different composer packages making up the framework, this makes it easier to know where to look.

And the idea was to have a message translator in the error handler, so framework errors can be localized (one of the remarks about v1). That is also why parameters are in brackets, so the error handler can extract them from the original message, and add them again to the translated message using sprintf().

@WanWizard
Copy link

WanWizard commented May 1, 2016

Other solutions are possible offcourse, like a generic error class that does the translations internally, but then you could approach it a bit differently, something like:

throw new FuelError('CMP-001')->setErrorInfo(['fullname' => $fullname]);

and

interface FuelErrorInterface
{
    public function setErrorInfo();
}

class FuelError extends \RuntimeException implements FuelErrorInterface
{
    protected $errorInfo = [];

    public function setErrorInfo($errorInfo)
    {
        $this->errorInfo = $errorInfo;
        return $this;
    }

    public function __toString() {
        // use the code passed to look up a localized message, and use the errorInfo array when formatting it
    }
}

@sagikazarmark
Copy link
Author

The idea was that messages in the code are in the first place descriptive for ourselfs, and not end-user messages.

I still feel that the maintainability is quite a problem here.

That is also why parameters are in brackets, so the error handler can extract them from the original message, and add them again to the translated message using sprintf().

Let's see the following exception message:

"Unable to load [$fullName]: Component [$component] is not available"

It would be translated into Hungarian like this:

"A [$component] komponens nem található, [$fullName] nem betölthető"

(In hungarian we have some semantical rules for cause-reason order)

In this case sprintf would not work since the order of parameters is changed. I am pretty sure that there are many other languages where it would cause a problem.

@WanWizard
Copy link

sprintf has no problem with positional parameters, this method was already (crudely) implemented in the previous v2 incarnation.

I don't have a problem with any alternative design you can come up with, as long as it meets the requirements:

  • easy to find in the code (with indication of which composer package to look into)
  • support for a variable list of parameters
  • fully translatable
  • low overhead and complexity, both in the code itself and in handling the translations

@emlynwest emlynwest added this to the 2.0.0-beta1 milestone Apr 18, 2017
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