diff --git a/webapp/channels/src/components/do_verify_email/do_verify_email.tsx b/webapp/channels/src/components/do_verify_email/do_verify_email.tsx index d26d1cc9aa..a741421e56 100644 --- a/webapp/channels/src/components/do_verify_email/do_verify_email.tsx +++ b/webapp/channels/src/components/do_verify_email/do_verify_email.tsx @@ -6,7 +6,7 @@ import {useIntl} from 'react-intl'; import {useSelector, useDispatch} from 'react-redux'; import {useLocation, useHistory} from 'react-router-dom'; -import {clearErrors, logError} from 'mattermost-redux/actions/errors'; +import {clearErrors, logError, LogErrorBarMode} from 'mattermost-redux/actions/errors'; import {verifyUserEmail, getMe} from 'mattermost-redux/actions/users'; import {getIsOnboardingFlowEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; @@ -91,7 +91,7 @@ const DoVerifyEmail = () => { dispatch(logError({ message: AnnouncementBarMessages.EMAIL_VERIFIED, type: AnnouncementBarTypes.SUCCESS, - } as any, true)); + } as any, {errorBarMode: LogErrorBarMode.Always})); trackEvent('settings', 'verify_email'); diff --git a/webapp/channels/src/components/user_settings/general/user_settings_general.tsx b/webapp/channels/src/components/user_settings/general/user_settings_general.tsx index f86f30aef9..0db241707a 100644 --- a/webapp/channels/src/components/user_settings/general/user_settings_general.tsx +++ b/webapp/channels/src/components/user_settings/general/user_settings_general.tsx @@ -11,6 +11,8 @@ import type {UserPropertyField} from '@mattermost/types/properties'; import type {UserProfile} from '@mattermost/types/users'; import type {IDMappedObjects} from '@mattermost/types/utilities'; +import type {LogErrorOptions} from 'mattermost-redux/actions/errors'; +import {LogErrorBarMode} from 'mattermost-redux/actions/errors'; import {Client4} from 'mattermost-redux/client'; import type {ActionResult} from 'mattermost-redux/types/actions'; import {isEmail} from 'mattermost-redux/utils/helpers'; @@ -111,7 +113,7 @@ export type Props = { maxFileSize: number; customProfileAttributeFields: IDMappedObjects; actions: { - logError: ({message, type}: {message: any; type: string}, status: boolean) => void; + logError: ({message, type}: {message: any; type: string}, options?: LogErrorOptions) => void; clearErrors: () => void; updateMe: (user: UserProfile) => Promise; sendVerificationEmail: (email: string) => Promise; @@ -324,7 +326,7 @@ export class UserSettingsGeneralTab extends PureComponent { this.props.actions.logError({ message: AnnouncementBarMessages.EMAIL_VERIFICATION_REQUIRED, type: AnnouncementBarTypes.SUCCESS, - }, true); + }, {errorBarMode: LogErrorBarMode.Always}); } } else if (err) { let serverError; diff --git a/webapp/channels/src/entry.tsx b/webapp/channels/src/entry.tsx index edbe885a42..8032020e89 100644 --- a/webapp/channels/src/entry.tsx +++ b/webapp/channels/src/entry.tsx @@ -4,7 +4,7 @@ import React from 'react'; import ReactDOM from 'react-dom'; -import {logError} from 'mattermost-redux/actions/errors'; +import {logError, LogErrorBarMode} from 'mattermost-redux/actions/errors'; import store from 'stores/redux_store'; @@ -42,8 +42,7 @@ function preRenderSetup(onPreRenderSetupReady: () => void) { stack: error?.stack, url, }, - true, - true, + {errorBarMode: LogErrorBarMode.InDevMode}, ), ); }; diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/errors.test.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/errors.test.ts index cf91c1108b..d0dd0f504f 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/errors.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/errors.test.ts @@ -3,11 +3,11 @@ import nock from 'nock'; -import {logError} from 'mattermost-redux/actions/errors'; +import {logError, LogErrorBarMode, shouldShowErrorBar} from 'mattermost-redux/actions/errors'; import {Client4} from 'mattermost-redux/client'; import TestHelper from '../../test/test_helper'; -import configureStore from '../../test/test_store'; +import configureStore, {makeInitialState} from '../../test/test_store'; describe('Actions.Errors', () => { let store = configureStore(); @@ -60,3 +60,26 @@ describe('Actions.Errors', () => { } }); }); + +test('shouldShowErrorBar', () => { + function makeTestState(enableDevMode: boolean) { + return makeInitialState({ + entities: { + general: { + config: { + EnableDeveloper: enableDevMode.toString(), + }, + }, + }, + }); + } + + expect(shouldShowErrorBar(makeTestState(false), {})).toBe(false); + expect(shouldShowErrorBar(makeTestState(true), {})).toBe(false); + expect(shouldShowErrorBar(makeTestState(false), {errorBarMode: LogErrorBarMode.Never})).toBe(false); + expect(shouldShowErrorBar(makeTestState(true), {errorBarMode: LogErrorBarMode.Never})).toBe(false); + expect(shouldShowErrorBar(makeTestState(false), {errorBarMode: LogErrorBarMode.Always})).toBe(true); + expect(shouldShowErrorBar(makeTestState(true), {errorBarMode: LogErrorBarMode.Always})).toBe(true); + expect(shouldShowErrorBar(makeTestState(false), {errorBarMode: LogErrorBarMode.InDevMode})).toBe(false); + expect(shouldShowErrorBar(makeTestState(true), {errorBarMode: LogErrorBarMode.InDevMode})).toBe(true); +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/errors.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/errors.ts index f7f9e7e71c..01c36bdaa5 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/errors.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/errors.ts @@ -6,6 +6,7 @@ import type {ErrorObject} from 'serialize-error'; import {LogLevel} from '@mattermost/types/client4'; import type {ServerError} from '@mattermost/types/errors'; +import type {GlobalState} from '@mattermost/types/store'; import {ErrorTypes} from 'mattermost-redux/action_types'; import {Client4} from 'mattermost-redux/client'; @@ -28,7 +29,36 @@ export function getLogErrorAction(error: ErrorObject, displayable = false) { }; } -export function logError(error: ServerError, displayable = false, consoleError = false): ActionFuncAsync { +export type LogErrorOptions = { + + /** + * errorBarMode controls how and when the error bar is shown for this error. + * + * If unspecified, this defaults to DontShow. + */ + errorBarMode?: LogErrorBarMode; +}; + +export enum LogErrorBarMode { + + /** + * Always show the error bar for this error. + */ + Always = 'Always', + + /** + * Never show the error bar for this error. + */ + Never = 'Never', + + /** + * Only shows the error bar if Developer Mode is enabled, and the message displayed will tell the user to check the + * JS console for more information. + */ + InDevMode = 'InDevMode', +} + +export function logError(error: ServerError, options: LogErrorOptions = {}): ActionFuncAsync { return async (dispatch, getState) => { if (error.server_error_id === 'api.context.session_expired.app_error') { return {data: true}; @@ -55,24 +85,29 @@ export function logError(error: ServerError, displayable = false, consoleError = } } - if (consoleError) { + if (options && options.errorBarMode === LogErrorBarMode.InDevMode) { serializedError.message = 'A JavaScript error has occurred. Please use the JavaScript console to capture and report the error'; } - const isDevMode = getState()?.entities.general?.config?.EnableDeveloper === 'true'; - let shouldDisplay = displayable; - - // Display announcements bar if error is a developer error and we are in dev mode - if (isDevMode && error.type === 'developer') { - shouldDisplay = true; - } - - dispatch(getLogErrorAction(serializedError, shouldDisplay)); + dispatch(getLogErrorAction(serializedError, shouldShowErrorBar(getState(), options))); return {data: true}; }; } +export function shouldShowErrorBar(state: GlobalState, options: LogErrorOptions) { + if (options && options.errorBarMode === LogErrorBarMode.Always) { + return true; + } + + if (options && options.errorBarMode === LogErrorBarMode.InDevMode) { + const isDevMode = state.entities.general.config?.EnableDeveloper === 'true'; + return isDevMode; + } + + return false; +} + export function clearErrors() { return { type: ErrorTypes.CLEAR_ERRORS, diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts index 76863d3718..60b48aba70 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts @@ -37,7 +37,7 @@ import {getCurrentUserId, getUsersByUsername} from 'mattermost-redux/selectors/e import type {ActionResult, DispatchFunc, GetStateFunc, ActionFunc, ActionFuncAsync, ThunkActionFunc} from 'mattermost-redux/types/actions'; import {isCombinedUserActivityPost} from 'mattermost-redux/utils/post_list'; -import {logError} from './errors'; +import {logError, LogErrorBarMode} from './errors'; // receivedPost should be dispatched after a single post from the server. This typically happens when an existing post // is updated. @@ -1325,7 +1325,7 @@ export function restorePostVersion(postId: string, restoreVersionId: string, con } catch (error) { // Send to error bar if it's an edit post error about time limit. if (error.server_error_id === 'api.post.update_post.permissions_time_limit.app_error') { - dispatch(logError({type: 'announcement', message: error.message}, true)); + dispatch(logError({type: 'announcement', message: error.message}, {errorBarMode: LogErrorBarMode.Always})); } else { dispatch(logError(error)); } diff --git a/webapp/channels/src/packages/mattermost-redux/test/test_store.js b/webapp/channels/src/packages/mattermost-redux/test/test_store.js index f93f1e60d7..569197fac0 100644 --- a/webapp/channels/src/packages/mattermost-redux/test/test_store.js +++ b/webapp/channels/src/packages/mattermost-redux/test/test_store.js @@ -3,6 +3,10 @@ import configureStore from 'mattermost-redux/store'; +export function makeInitialState(preloadedState) { + return testConfigureStore(preloadedState).getState(); +} + export default function testConfigureStore(preloadedState) { const store = configureStore({preloadedState, appReducers: {}, getAppReducers: () => {}});