[Problem] Raw exception text displayed untranslated to end users

There are about 50 occurrences of the string QString::fromLatin1(e.what()) in our C++ codebase -- that is, exception text being converted into a string that is designed to be displayed to the end user, usually via a modal dialog. In most cases, the text of those exceptions is untranslated. In many cases, the text isn't really intended for end users at all, but contains information predominantly for developers. This count ignores similar problems in Python (which are harder to find using a simple search like this).

As a general rule, I don't think we should show exception text to end users. If there is some action they can take to fix things, then we should write a user-facing error dialog. Otherwise, the exception text should be logged, and a generic error displayed to the user. @yorikvanhavre, @abdullahtahiriyo what do you think of that strategy?

Sorry for the long post.

I think we should tackle this issue together with this other: https://forum.freecad.org/viewtopic.php?p=682642#p682642

Needless to say I am not English native. I often use localisation when I want to improve one other language. One of the things I dislike most is when a log output is mixed language. I want the UI in the localised language, but the logs in English. It is very frustrating to try to get or provide help when one uses translated error messages from a log. I see user @kaktusus active here too, and I follow his other topic with the EditDialog not localised pop-up. He may also give his opinion about localising logs. I presume he uses a Polish localisation and searching for a solution with a Polish localised message may be daunting.

I understand the "Report View" as a log. To me it does not qualify as "user facing" (as opposed to the UI or the notification area, which is indeed user facing).

For that reason, I designed both the exception translation mechanism and the console extension for the notification area with two prerequisites:

That the log in the report view stays in English, while the notification area, or a pop-up if intrusive notifications are configured, is localised).
To avoid translation dependencies in the App part of FreeCAD

While I am reticent to add the translation framework dependencies, I understood from the thread that the link dependency is (at least now) already there and I may be just wrong to set it as a prerequisite. I accept this.

You rightly point out in that thread that the richest information for the generation of a message, let it be an exception or a console message, it at the source. I agree. However, translating at the source, at least how we use the framework now, has the inherent perceived problem that the log will be localised.

One option, at least in some cases, could be to trigger two notifications/logs, one in English only to the log and only the log, and another one translated at the source intended for the UI (The report view does not listen to Notifications or TranslatedNotifications, only to Error and Warning, whereas the NotificationArea listen to all, depending on the configuration). This has more overhead, but it is probably the only option in the case of messages involving dynamic strings, if we keep the premise that the log is only in English.

Most messages are currently not dynamically generated strings, but compile time strings. For this case, if we do not translate at the source, but just mark the string with the translation macro, can lead to a notification in English in the log, and a localised notification in the UI. For this to work, the context needs to be "Notifications", as the framework in place for pop-ups or NotificationArea (depending on user preferences), attempts translation with this context.

This does not answer your questions directly, but I think it will help set the context.

that is, exception text being converted into a string that is designed to be displayed to the end user, usually via a modal dialog

I think we should generally respect the configuration of the NotificationArea. If intrusive messages are ON, then indeed a modal dialog is the way to go. If they are OFF, then we should use the NotificationArea. This is automatically achieved if the Notifications framework is used instead of a direct call to QMessageBox (in those situations where the only option the user has is to click 'ok'):

https://github.com/FreeCAD/FreeCAD/blob/5f82aa42bb1eda8084b36ccd58fb94974ecdb1c6/src/Gui/Notifications.h#L36

https://github.com/FreeCAD/FreeCAD/blob/5f82aa42bb1eda8084b36ccd58fb94974ecdb1c6/src/Mod/Sketcher/Gui/CommandConstraints.cpp#L4347

Of course, this framework can be extended if it does not adapt to all situations. But I think we should use just one framework consistently.

In most cases, the text of those exceptions is untranslated.

Exceptions are/were translated using the Exceptions context in the previous framework. If an exception is a notification and needs to be translated, I think we should move to the Notifications framework and use the Notifications context.

In any case, mixed context works too, but requires invoking the translation manually, whereas with the Notifications context the process is transparent and automatic.

In many cases, the text isn't really intended for end users at all, but contains information predominantly for developers.

If an exception is not a intended as a notification to the user, it should not be presented to the user and IMO should not be translated. It should be send to the Log untranslated (IMO, but it may need more discussion).

This count ignores similar problems in Python (which are harder to find using a simple search like this).

I agree. We have the case of SketchObjectPyImp in the forum as an example. We should think of a class of solution covering this corner too, even if implementation may not come at once, but in stages.

As a general rule, I don't think we should show exception text to end users. If there is some action they can take to fix things, then we should write a user-facing error dialog.

In principle I agree.

However (and this is the main reason of giving context above and the very long post): (i) the richest amount of information is generally at the source [This is your argument too]. For example if errors can be for different reasons, there may be some error codes at the source for the different reasons. If they affect different elements and this information is only available at the source (e.g. it affects constraint with index 5)

(ii) the exception framework mostly provides only for one string being conveyed over to where the exception is caught.

So, in the case of exceptions that should trigger meaningful notifications for the user, as the user can fix things, we need to convey the best information to the user. In this case a generic error message ("take a look to the (English) log") should not be it (IMO).

To convey information from the exception originating point to where it is caught, we have two options: (a) to use a string (ii), which is a very simple solution, but it may not cover all corners (see dynamically generated strings and translations). (b) to extend the exceptions to convey additional information (specific exception types, generic fields in exception such an error code, a variant, ...), which is more complex. The first one, the specific exception type, has the additional problem that there is not a Python counterpart in Python stock implementation (As opposed to e.g. ValueError). We often need Python exceptions that will afterwards turn into c++ Exceptions when (re)-entering c++ land.

Otherwise, the exception text should be logged, and a generic error displayed to the user.

I agree. As it is not to be shown to the user, it will not be translated. It may even be untranslatable (e.g. a trace, or errors from the OCCT kernel). However, I think we should still strive to make the generic error messages as specific as possible. In the PD issue you link, it would not be extremely useful to say (localised) "An error has occurred" either. If an exception at that position can be generated for different reasons, additional information is necessary to show the right message.

Because of the impact it has on the notification framework, I think that the first thing we need to clarify is if the log should use mixed language or should be pure English. Any extension of the framework should take this decision as a requisite. In other words, I do not think it is sound to try to find an strategy without taking this decision first.

I really like how you presented the problem.

You know ... sometimes you need to expand on a statement to make your point clear. So in my opinion, you don't have to apologize for making your point of view clear and interesting. :wink:

Leaving aside all the issues and problems that accompany programmers, I will say that I think it is reasonable to leave the original English text in the report view window.

I also really like your approach that everything shown outside it should be localized into the user's language.

I understand that solving some of the GUI translation issues can be complicated especially at this stage when not so much attention was paid to translations before.

I will read again everything you wrote carefully, including the presented topic from the forum. Now I have to get on with life ... my wife is chasing me to go shopping because she is missing some products and wants to prepare a meal. :rofl:

just one thing holds me back, we should take care of not taking the autodesk path with maddening generic messages like "an unknown error has occured. Revit will now close.". Displaying untranslated exception messages may look not too clean, but it's IMHO infinitely better to show more info than less info. I also agree with Abdullah that the report window is a place where you want to see those messages. Maybe we need something on top of the existing, like, aside from Console.PrintError(), a Console.PrintUserError()?

Thank you for your well-considered comments, @abdullahtahiriyo -- they are appreciated, as always. I think a move to the Notification framework you have developed would be a welcome step in the right direction: in the C++ code at any rate it's pretty easy to find these exceptions that are being directly displayed to the user in a modal dialog (there are only about 50 of them). So it sounds like those can be directly migrated into your framework.

I am happy to leave Report View untranslated. What is the process for writing something to Report View that does not also display in the Notifications area?

@chennes

I am very grateful to you for pursuing this topic .

The framework currently in master is limited. It works like this:

The report view and the notification area, both subscribe to error and warning (by default). The notification area subscribes additionally to notifications and translated notifications (these are all messages of Base::Console). Errors, warnings and notifications carry untranslated strings. The Report View will not attempt any translation. The Notification Area will attempt a translation with context "Notifications". Translated notifications carry already translated messages and will only appear on the NotificationArea, because it is the only observer subscribed to this message type.

If the user configures it not to see errors or warnings in the Notifications Area, then they will not appear there.

This means that this framework does not have a fixed mechanism to do exactly what you ask.

In addition, when a message is not intended for the Report View, it can only be a Notification or TranslatedNotification. This is suboptimal as there is no way to convey information on severity (error, warning,...).

To complicate it even more, there is no way to mark something as "error intended for developers" when using the Consoler.Error. The Interpreter sends messages for any Python exception during execution. These, stack traces, end up in the notification area annoying users.

Based on the input of the last month, I am currently extending the Console error system and the notification framework so that we have much more control. I am working here: https://github.com/abdullahtahiriyo/FreeCAD_sf_master/tree/_console_improvement_

Basically, the TranslatedNotification message disappears and the rest of the messages are provided with metadata context:

An indication whether the message is intended for Users, Developers or Both.
An indication whether the message is already translated, it is untranslated (but could be translated). or it is untranslatable (such as a dynamic string with no counterpart or a trace).

The TranslatedNotification message is implemented via a Notification with the ContentType::Translated.

I am still testing the framework and looking at corner cases. But to answer your question:

In this framework, the ReportView will not process messages that are Translated or are only intended for the User. The NotificationArea will not process messages that are untranslatable or that are only intended for the Developer. Notifications are only treated by the NotificationArea (as the ReportView is not subscribed to them).

Both Base::Console() and Notifications.h have one templated function that can generate any combination of these metadata. However, they also provide convenience functions.

For the Gui part of FreeCAD, Notifications.h provides:

inline void NotifyWarning(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void NotifyUserWarning(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void TranslatedUserWarning(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void NotifyError(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void NotifyUserError(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void TranslatedUserError(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void TranslatedNotification(TNotifier && notifier, TCaption && caption, TMessage && message);
inline void Notification(TNotifier && notifier, TCaption && caption, TMessage && message);

These functions do not accept (will static_assert and provide a meaningful error message during compilation) Developer only messages or messages that are untranslatable. They are intended for the Gui and thus to show localised messages to the user. They respect the preferences if they should be shown intrusively (pop-up) or via the Notification Area.

This separation allows now to send errors and warnings only to the Notification Area via NotifyUserXXXX or TranslatedUserXXXX depending on whether the string is already translated or not. Before only Notifications (with the 'i' icon) could be sent exclusively to the NotificationArea. The versions without "User" send the message to both NotificationArea and ReportView (they take an untranslated string which will be shown directly by the latter and will be translated with the Notifications context by the former before showing).

The App part can use the Base::Console convenience functions:

    inline void Warning (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void DeveloperWarning (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void UserWarning (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void TranslatedUserWarning (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void Error   (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void DeveloperError (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void UserError (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void TranslatedUserError (const std::string & notifier, const char * pMsg, Args&&... args);
    inline void UserNotification( const std::string & notifier, const char * pMsg, Args&&... args );
    inline void UserTranslatedNotification( const std::string & notifier, const char * pMsg, Args&&... args );

This allows, for example, to send an error only to the ReportView by using Console().DeveloperError. It won't appear on the Notification Area. Only to the NotificationArea, via Console().UserError. Or to both via Console().Error. When the App part includes the dependency of the translation framework, it is also possible to send a translated error, via Console().TranslatedUserError. The Report view won't show translated strings. This would appear exclusively in the NotificationArea (or any future observer which handles this situation).

This also enables to send two errors at a given point. One intended for the report view and one intended for the notification area. This could be an option for the case of the untranslated dynamic strings.

I am still looking at how to best handle exceptions. I will post later on

Now. What we have is the first system and we are in feature freeze. Eventually a decision will need to be taken whether we regard this as "bug fixing" and take the risk of delaying the release or introducing new bugs; or whether we merge it after release and leave with what we have.

This sounds excellent -- my understanding is that it will take some deliberate developer effort to migrate our error reporting to this, so that even if we merged your new code during the freeze, we'll still basically be stuck with the old error message handling until developers go into their code and rework it. Is that right?

It will be a big step for a better tomorrow. :wink:

There is quite some truth in your words. There are some remarkable exceptions.

The most important one is being able to substitute the call of ReportException in the Interpreter. With the current framework every exception gets into the NotificationArea (unless all errors are disabled). One user reported getting so annoyed that he disabled all errors in the notification area just not to see them (and thus missing all other errors). The new framework will come with switching that reporting to Developer and thus won't appear in the NotificationArea. One single function change for a substantial benefit. We will have over one year of complaints from annoyed users because of this if we don't. I think this we should carefully weigh.

The Sketcher is mostly migrated to non-intrusive messages and what is not yet will be before release (got a PR pending). In this pending PR there is a loss of severity information for using the old framework (the reason why it is not merged). With the new framework, severity would be kept. Yes, we still have some corner cases (dynamic strings). Perhaps this one is not fixed before release. I do not know.

Irrespective of the current situation (close to release), new or old framework, other WBs need to be migrated and Developer time would be necessary. Unless we do some targeted effort, it won't happen overnight or it won't happen at all. It is not the most exciting development area. It is however one task where it is very difficult to introduce a bug when switching the call (unless the framework itself has a bug). Using the new framework is much more straightforward as the severity is kept, what was a warning keeps being a warning as opposed to needing to change it to a notification (and having the funny feeling on whether this is the right call or not).

Generally speaking, when adapting new code, I also think there is a kind of 20%-80% rule to it.

For most pop-ups where the user can only say "ok", one can grep the source of QMessageBox and because the notification framework uses the same syntax (arguments), it is just adding the header and changing the call (maintaining the arguments). This is should be painless. It was for Sketcher at least. Then all (former) pop-ups will or that won't respect the non-intrusive settings

The problem will arise with the remaining 20% cases, exceptions and strings that are simply not translated, or became dynamic and cannot be translated. This requires the 80% of the effort and definitely it won't happen before release. Conversely, advancing the translations will increase the visibility of that 20%. It may even encourage further fixing during the 0.22 development cycle, porting them for point release should be possible too.

Embracing that we know we won't be able to make everything right before release, we need to decide how much makes sense to do it before. I do not even know how much time we have left before release. End of May?

I set a deadline of 31 May for the Splashscreen entries, plus a week of voting after that, so our earliest date for the release is June 7 or so: at that time we will have to look at the outstanding bug reports slotted as "required" for 0.21 and decide if they can be dealt with, or no.

@chennes

I have PR-ed it.

Today I have been testing it with the Sketcher and with direct statements from Python, and adapting the preferences of the Notification Area.

I have coded it over the weekend. I have requested your review, but I will be also code reading it myself. Generally I do that before PR-ing it. But I wanted to make the code available as soon as possible.

I will also continue adapting the parts of the sketcher that are currently not yet using this new framework in a separate PR. This should allow us to identify any shortcoming or bug.

Let me know what you think.

It is now merged. Thanks for the review.

I will work this week on improving Sketcher messages and see which problems arise. Feel free to share here also your efforts or if you find some corner cases.

We still have the dynamic strings issue. Werner posted in the forum with an idea to store the formatting string and the arguments. After migrating the "normal" messages I will take a look to see if I can make that idea work.

When working the GUI messages for the Sketcher, I am following this procedure:

1. Notifier

Notifier can be any string that identifies the origin. However, I am using the DocumentObject label when available, if not the Document label.

why?

Because FreeCAD will be multi-document and will only have one Notifications Area. If every message identifies the document or even better the document object, then it is much more easier to know which document is sending the messages.

One can think this is very easy to know, as one is doing it. Think of a recompute over several documents. That may sound different... which sketch is actually failing from all the sketches in all documents?

The Notifications framework (Gui/Notifications.h) is there to simplify the life of the developer, it will take as notifier any of a App::DocumentObject, Gui::Document, App::Document, Gui::ViewProviderDocumentObject (other than a string), and will produce the right notifier string.

2. GUI code should only use Console directly exceptionally

If the objective of the code is to show a message of a user mistake (which can be a pop-up or a non-intrusive notification), it should use the Notification Framework (Notifications.h). The reason is because this framework is able to follow the preferences in settings to show a pop-up or non intrusive message.

If it is an error resulting from a python exception, the exception has already been reported by Interpreter.h. If any message is to be shown to the user, it should be only to the user (the developer has already been notified in the report view).

Exceptionally, there may be a case where both need to be notified. In this case, the Notifications Framework will handle the right settings (to the report view and pop-up, or to the report view and the Notification Area).

Very much exceptionally, there may be a cases where only the developer needs to be notified. Then, using console is the right choice.

3. Message type I: User mistake (e.g. did not select the right elements)

Originally they were mostly QMessageBox::warning (also info or error sometimes).

I substitute with a TranslatedUserWarning (TranslatedNotification for info, TranslatedUserError for error), like this:

Gui::TranslatedUserWarning(Obj, QObject::tr("Wrong selection"),
                                 QObject::tr("Select the right things from the sketch."));

Sometimes there is no DocumentObject (e.g. SketchObject) available. I then use the document as notifier:

Gui::TranslatedUserWarning(getActiveGuiDocument(), 
                           QObject::tr("Wrong selection"),
                            QObject::tr("Select an edge from the sketch."));

4. Message type II: a python command brought an exception back

This is generally an error.

In this pass I am not looking at the actual strings in the source. That will come in a second pass.

If the code assumes the string can be directly translated, I use a Gui::TranslatedUserError, as in:

Gui::TranslatedUserError(Obj,
                QObject::tr("Error"),
                QObject::tr(getStrippedPythonExceptionString(e).c_str()));

If the code just notifies the string without translation, or uses a Console.Error() call, I change it to use a Gui::NotifyUserError as in:

Gui::NotifyUserError(Obj,
         QT_TRANSLATE_NOOP("Notifications", "Invalid Constraint"),
         e.what());

Many of these won't show a translation now if the strings are dynamic or not actually translated. That is for a second pass.

5. Message type II: a python command brought an exception back, but the code adds a new meaningful error

Sometimes it may be that just one of the suboperations fails, but the code wants to notify something more meaningful that originates as consequence.

The exception was notifier to the developer. However the meaningful message was not. Then it is best not to directly translate the string, but mark it for translation, and use the NotifyError function, which reports to both developer and user. This way, the developer gets the untranslatable exception and the English version of the meaningful error. The user gets the translated meaningful error. An example:

Gui::NotifyError(Obj,
                                    QT_TRANSLATE_NOOP("Notifications", "Error"),
                                    QT_TRANSLATE_NOOP("Notifications", "Cannot create arc of hyperbola from invalid angles"));

6. An exception can be originated by c++ or python code

If at the moment of catching one exception the exception originated in Python, it has been reported to the developer (Interpreter.cpp does). However, some times the exception may not have passed by Python and it is not reported.

Exceptions have in-built a double-reporting prevention mechanism. In this cases it is always save to do e.ReportException(). If it was not reported, this will report it to the developer once. If it was already reported, then it is ignored.

In addition of this, if the user may need to be notified with a meaningful error, as indicated above (for example using Gui::NotifyError, which will ensure that both developer and user have a good idea of what is happening. The Developer will get context of during which operation the exception happened).

I thought you may find this useful. If there is anything that does not feel right, or you have other ideas, let me know.

P.S.: I keep editing this post to add new information. Others feel free to add extra information too. Perhaps we can eventually make this into some other text, wiki, handbook,...

A first package of non-intrusive notifications using the new framework: https://github.com/FreeCAD/FreeCAD/pull/9604

A second package splitting errors and warnings sent directly to the console, so that meaningful ones get to the user and developer only ones get only to the developer:

https://github.com/FreeCAD/FreeCAD/pull/9652

As you can see, others have also faced this problem.

And they found a very comfortable solution. The entire interface has a translation and ... and a button which additionally allows you to copy the message in English to the clipboard. This is a very comfortable solution.

I also remember the comments of one forum user on this topic. He had very factual arguments. He recounted that it was easier to find the solution to a particular error by pasting the error text in English into a google search. Therefore, he remained in opposition to translating error messages.

The solution I presented brings us closer to the ideal. It remains to be clarified whether it is worth implementing them.

FreeCAD / FreeCAD-translations

[Problem] Raw exception text displayed untranslated to end users #223