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

Localizable exceptions #135

Closed
valzav opened this issue Jul 6, 2015 · 10 comments
Closed

Localizable exceptions #135

valzav opened this issue Jul 6, 2015 · 10 comments
Assignees
Labels

Comments

@valzav
Copy link

valzav commented Jul 6, 2015

It would be nice to have exceptions that we can translate to other languages in GUI.
In order to do this an exception may require to have some code or symbol and parameters, e.g. exception object might look like this:
{
error: "not-valid-address",
param1: "1EDJEW3508324LJDSF908342L",
eng: "Not valid address: %1",
message: "Not valid address: 1EDJEW3508324LJDSF908342L",
stack: ...
}

@valzav valzav changed the title Localizable exception Localizable exceptions Jul 6, 2015
@clayop
Copy link
Contributor

clayop commented Jul 6, 2015

This is what I really need. The word order of Korean is opposite to English, so it is very hard to translate the wallet to Korean.

@emfrias
Copy link
Collaborator

emfrias commented Jul 6, 2015

On the c++ side, fc::exceptions are already use name-value pairs for parameter substitution, so word order differences shouldn't be a problem.

See the 'format' and 'data' in the last exception I generated:
Source:

      FC_THROW("Invalid private key ${key}", ("key", wif_key));

JSON:

{
    "code": 0,
    "name": "exception",
    "message": "unspecified",
    "stack": [
        {
            "context": {
                "level": "error",
                "file": "wallet.cpp",
                "line": 1915,
                "method": "import_key",
                "hostname": "",
                "thread_name": "th_a",
                "timestamp": "2015-07-06T19:57:19"
            },
            "format": "Invalid private key ${key}",
            "data": {
                "key": "this_is_not_a_key"
            }
        }
    ]
}

@vikramrajkumar vikramrajkumar added this to the beta milestone Jul 6, 2015
@valzav
Copy link
Author

valzav commented Jul 7, 2015

Ok, so we already have name-value parameters. What about error code or symbol? This would be more convenient than parsing error text on a GUI side.
Probably the easiest way to add it would be via additional parameter:
FC_THROW("Invalid private key ${key}", ("code":"invalid_private_key")("key", wif_key));

@bytemaster
Copy link
Contributor

Code and name are the first two parameters of the exception object. Ben is currently replacing FC_ASSERT with named exceptions with proper error codes.

@emfrias
Copy link
Collaborator

emfrias commented Jul 7, 2015

... beat me to it.

we sort of have a code, but it's not exactly what you want. The codes are embedded in the exception object. In the above example, code was 0 because I used FC_THROW to throw the base class. If I had thrown a specific derived exception, it would have had a meaningful code.

Codes are defined here for the blockchain, they're that number around 30000+:
https://github.com/cryptonomex/graphene/blob/master/libraries/chain/include/graphene/chain/exceptions.hpp#L25-L113

but right now it's common for us to throw the same type of exception in two different places, using two different messages and two sets of key-value pairs. Unless we change the way we throw exceptions, the code won't be enough for translation.

I'm only familiar with gettext, and I think it uses the english string itself for looking up the translations, that might be the the most reliable thing to do.

@vikramrajkumar
Copy link
Contributor

Related: #113

@valzav
Copy link
Author

valzav commented Jul 7, 2015

@emfrias - good point, I think we need to standardize messages and parameters for all exceptions with the same error code. Maybe add them to FC_DECLARE_DERIVED_EXCEPTION statements?

@theoreticalbts
Copy link
Contributor

I should have some code sometime tomorrow. My approach, discussed in #113, is to ideally have each individual place with an exception throw a different one. The result will be several exceptions with identical parameters and error messages. We may wish to collapse these to a single exception for translation purposes.

I propose using a subclassing mechanism to accomplish this collapse. The localization framework should have a way to associate an error message with a particular exception type. When given an exception of derived type D, you check in types in this order: D, parent(D), parent(parent(D)), ..., stopping when you find a type for which there is a message in the current language.

I believe I can write a program to use fc reflection to climb the inheritance tree, resulting in a map from exception code to a list of parent classes. This allows the database to be independent of exception code changes (in the numbering scheme currently proposed, the operation code is included in the exception code, so if operations are re-ordered, exception codes for the reordered operations would change).

@valzav
Copy link
Author

valzav commented Jul 7, 2015

@theoreticalbts sounds good to me.. but I have a feeling that there might be an easier solution that doesn't involve subclassing. Also if translation is missing in some language we can just default to english.

@vikramrajkumar
Copy link
Contributor

This issue was moved to bitshares/bitshares-core#37

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

No branches or pull requests

6 participants