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

Unify system error messages #4542

Merged
merged 3 commits into from Aug 21, 2018
Merged

Unify system error messages #4542

merged 3 commits into from Aug 21, 2018

Conversation

denis-anisimov
Copy link
Contributor

@denis-anisimov denis-anisimov commented Aug 18, 2018

Fixes #3777


This change is Reviewable

Copy link
Contributor

@pekam pekam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained


flow-client/src/main/java/com/vaadin/client/SystemErrorHandler.java, line 116 at r1 (raw file):

     */
    public void handleError(String errorMessage) {
        Console.error(errorMessage);

This is now logged twice: once here and again when calling the new handleError(caption, message, details).

You can move this line inside the if-block below to still log it in production mode.

Copy link
Contributor

@pekam pekam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained


flow-client/src/main/java/com/vaadin/client/SystemErrorHandler.java, line 116 at r1 (raw file):

Previously, pekam (Pekka Maanpää) wrote…

This is now logged twice: once here and again when calling the new handleError(caption, message, details).

You can move this line inside the if-block below to still log it in production mode.

done

Copy link
Contributor

@caalador caalador left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained, and 1 stale

Copy link
Contributor

@caalador caalador left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r3.
Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained, and 1 stale

@pekam pekam merged commit db73c7b into master Aug 21, 2018
@pekam pekam deleted the 3777-system-error branch August 21, 2018 08:23
@pekam pekam added this to the 1.1.0.beta3 milestone Aug 21, 2018
pekam pushed a commit that referenced this pull request Aug 21, 2018
* Unify system error messages

Fixes #3777

* Remove duplicate logging call

* Fix a test by finding only direct children of body with class 'message'
pekam pushed a commit that referenced this pull request Aug 21, 2018
* Unify system error messages

Fixes #3777

* Remove duplicate logging call

* Fix a test by finding only direct children of body with class 'message'
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

Successfully merging this pull request may close these issues.

None yet

3 participants