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

Color-formatted messages thrown as exceptions #402

Open
NadirRoGue opened this issue May 11, 2022 · 4 comments
Open

Color-formatted messages thrown as exceptions #402

NadirRoGue opened this issue May 11, 2022 · 4 comments

Comments

@NadirRoGue
Copy link
Contributor

Hellow:
The following code format error messages with colors:

const std::map<ErrorLevel, std::string> COLOR{{ErrorLevel::INFO, "\033[1;34m"},

These messages are being thrown as exceptions. Brayns captures these exceptions to send them back to the connected clients as informative messages, however, they contain the color-format characters, as follows:

Result: {
    "error": {
        "code": -32603,
        "data": null,
        "message": "\n\u001b[1;31m/gpfs/bbp.cscs.ch/release/l2/2012.07.23/morphologies/ascii/dend-tkb070202a2_ch0_cc2_n_tb_100x_3_axon-tkb061126a4_ch0_cc2_h_zk_60x_1_-_Clone_5.asc:1:error\u001b[0m\nCan't iterate past the end"
    },
    "id": "ID-4",
    "jsonrpc": "2.0"
}

I believe it would be better to throw the exceptions unformatted, and maybe add the format if MorphIO decides to log the message to some output.

@mgeplf
Copy link
Contributor

mgeplf commented May 11, 2022

I believe it would be better to throw the exceptions unformatted,

Agree 100%; the library is being not helpful by formatting this.

@NadirRoGue
Copy link
Contributor Author

I'll take care of it. I believe, since it is an I/O library, it should not log errors, but rather just throw the exception and let the calling application handle it. Is that an acceptable behaviour?

@mgeplf
Copy link
Contributor

mgeplf commented May 12, 2022

Is that an acceptable behaviour?

Hold off a moment on that; I need to remember why it was done this way, as opposed to throwing exceptions; I believe there are some 'errors' that are 'acceptable', but I need to look into it more.

I think it's worth discussing and figuring out how better to handle errors. At the momemnt there is a static somewhere which I really dislike.

@NadirRoGue
Copy link
Contributor Author

NadirRoGue commented May 12, 2022

Yes, there is a list of ignored warnings that is static.
I'd say we could define an interface of the likes

class IErrorHandler
{
public:
    virtual ~IErrorHandler() = default;
    
    virtual void report(Warning warning, const std::string &message) = 0;
    
    virtual void report(Warning warning, readers::ErrorLevel level, const std::string &message) = 0;
};

Then, by default, it would be initialized to a MorphIO implementation that emulates the current behaviour (and would have a non static member list of ignored warnings), but would allow the caller to modify the behaviour by providing a custom implementation.

What do you think? Maybe this solution is too tailored to my use case, I am not sure if its too big of a change.

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

No branches or pull requests

2 participants