From f2957d4885f349c668a0d32fbb770d6bb78c609b Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Thu, 31 Dec 2020 17:01:49 +0500 Subject: [PATCH] Login: Add error handling for 500 status code This handles the cases where some internal server error occurs and no error message is shown to users. VAN-95 --- src/logistration/LoginFailure.jsx | 13 ++++++- src/logistration/data/constants.js | 3 +- src/logistration/data/sagas.js | 9 ++++- src/logistration/data/tests/sagas.test.js | 38 ++++++++++++++++--- src/logistration/tests/LoginFailure.test.jsx | 18 ++++++++- .../__snapshots__/LoginFailure.test.jsx.snap | 22 +++++++++++ 6 files changed, 92 insertions(+), 11 deletions(-) diff --git a/src/logistration/LoginFailure.jsx b/src/logistration/LoginFailure.jsx index e93db9cd..c30d2530 100644 --- a/src/logistration/LoginFailure.jsx +++ b/src/logistration/LoginFailure.jsx @@ -5,7 +5,7 @@ import { Alert, Hyperlink } from '@edx/paragon'; import PropTypes from 'prop-types'; import { processLink } from '../data/utils/dataUtils'; -import { INACTIVE_USER, NON_COMPLIANT_PASSWORD_EXCEPTION } from './data/constants'; +import { INACTIVE_USER, INTERNAL_SERVER_ERROR, NON_COMPLIANT_PASSWORD_EXCEPTION } from './data/constants'; import messages from './messages'; const LoginFailureMessage = (props) => { @@ -60,6 +60,17 @@ const LoginFailureMessage = (props) => { ); break; } + case INTERNAL_SERVER_ERROR: + errorList = ( +
  • + +
    • + ); + break; default: // TODO: use errorCode instead of processing error messages on frontend errorList = value.trim().split('\n'); diff --git a/src/logistration/data/constants.js b/src/logistration/data/constants.js index ecba4d1a..342730dd 100644 --- a/src/logistration/data/constants.js +++ b/src/logistration/data/constants.js @@ -1,2 +1,3 @@ -export const NON_COMPLIANT_PASSWORD_EXCEPTION = 'NonCompliantPasswordException'; export const INACTIVE_USER = 'inactive-user'; +export const INTERNAL_SERVER_ERROR = 'internal-server-error'; +export const NON_COMPLIANT_PASSWORD_EXCEPTION = 'NonCompliantPasswordException'; diff --git a/src/logistration/data/sagas.js b/src/logistration/data/sagas.js index 4abd79dc..72dc27a8 100644 --- a/src/logistration/data/sagas.js +++ b/src/logistration/data/sagas.js @@ -2,6 +2,7 @@ import { call, put, takeEvery } from 'redux-saga/effects'; import { camelCaseObject } from '@edx/frontend-platform'; import { logError } from '@edx/frontend-platform/logging'; +import { INTERNAL_SERVER_ERROR } from './constants'; // Actions import { @@ -67,8 +68,12 @@ export function* handleLoginRequest(action) { )); } catch (e) { const statusCodes = [400]; - if (e.response && statusCodes.includes(e.response.status)) { - yield put(loginRequestFailure(camelCaseObject(e.response.data))); + if (e.response) { + if (statusCodes.includes(e.response.status)) { + yield put(loginRequestFailure(camelCaseObject(e.response.data))); + } else { + yield put(loginRequestFailure(camelCaseObject({ errorCode: INTERNAL_SERVER_ERROR }))); + } } logError(e); } diff --git a/src/logistration/data/tests/sagas.test.js b/src/logistration/data/tests/sagas.test.js index d75d114b..a9b2c1f9 100644 --- a/src/logistration/data/tests/sagas.test.js +++ b/src/logistration/data/tests/sagas.test.js @@ -235,7 +235,7 @@ describe('handleLoginRequest', () => { }); it('should call service and dispatch error action', async () => { - const loginErrorRespinse = { + const loginErrorResponse = { response: { status: 400, data: { @@ -244,7 +244,7 @@ describe('handleLoginRequest', () => { }, }; const loginRequest = jest.spyOn(api, 'loginRequest') - .mockImplementation(() => Promise.reject(loginErrorRespinse)); + .mockImplementation(() => Promise.reject(loginErrorResponse)); const dispatched = []; await runSaga( @@ -257,7 +257,33 @@ describe('handleLoginRequest', () => { expect(loggingService.logError).toHaveBeenCalled(); expect(dispatched).toEqual([ actions.loginRequestBegin(), - actions.loginRequestFailure(camelCaseObject(loginErrorRespinse.response.data)), + actions.loginRequestFailure(camelCaseObject(loginErrorResponse.response.data)), + ]); + loginRequest.mockClear(); + }); + + it('should handle 500 error code', async () => { + const loginErrorResponse = { + response: { + status: 500, + data: { + errorCode: 'internal-server-error', + }, + }, + }; + + const loginRequest = jest.spyOn(api, 'loginRequest').mockImplementation(() => Promise.reject(loginErrorResponse)); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleLoginRequest, + params, + ); + + expect(dispatched).toEqual([ + actions.loginRequestBegin(), + actions.loginRequestFailure(camelCaseObject(loginErrorResponse.response.data)), ]); loginRequest.mockClear(); }); @@ -302,7 +328,7 @@ describe('handleNewUserRegistration', () => { }); it('should call service and dispatch error action', async () => { - const loginErrorRespinse = { + const loginErrorResponse = { response: { status: 400, data: { @@ -311,7 +337,7 @@ describe('handleNewUserRegistration', () => { }, }; const registerRequest = jest.spyOn(api, 'registerRequest') - .mockImplementation(() => Promise.reject(loginErrorRespinse)); + .mockImplementation(() => Promise.reject(loginErrorResponse)); const dispatched = []; await runSaga( @@ -324,7 +350,7 @@ describe('handleNewUserRegistration', () => { expect(loggingService.logError).toHaveBeenCalled(); expect(dispatched).toEqual([ actions.registerNewUserBegin(), - actions.registerNewUserFailure(loginErrorRespinse.response.data), + actions.registerNewUserFailure(loginErrorResponse.response.data), ]); registerRequest.mockClear(); }); diff --git a/src/logistration/tests/LoginFailure.test.jsx b/src/logistration/tests/LoginFailure.test.jsx index 1a7b59f1..76c0bb0a 100644 --- a/src/logistration/tests/LoginFailure.test.jsx +++ b/src/logistration/tests/LoginFailure.test.jsx @@ -3,7 +3,7 @@ import renderer from 'react-test-renderer'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import LoginFailureMessage from '../LoginFailure'; -import { INACTIVE_USER, NON_COMPLIANT_PASSWORD_EXCEPTION } from '../data/constants'; +import { INACTIVE_USER, INTERNAL_SERVER_ERROR, NON_COMPLIANT_PASSWORD_EXCEPTION } from '../data/constants'; describe('LoginFailureMessage', () => { let props = {}; @@ -45,6 +45,22 @@ describe('LoginFailureMessage', () => { expect(tree).toMatchSnapshot(); }); + it('should match internal server error message snapshot', () => { + props = { + loginError: { + errorCode: INTERNAL_SERVER_ERROR, + }, + }; + + const tree = renderer.create( + + + , + ).toJSON(); + + expect(tree).toMatchSnapshot(); + }); + it('should match direct render of error message snapshot', () => { props = { loginError: { diff --git a/src/logistration/tests/__snapshots__/LoginFailure.test.jsx.snap b/src/logistration/tests/__snapshots__/LoginFailure.test.jsx.snap index fc727770..527de7cb 100644 --- a/src/logistration/tests/__snapshots__/LoginFailure.test.jsx.snap +++ b/src/logistration/tests/__snapshots__/LoginFailure.test.jsx.snap @@ -89,6 +89,28 @@ exports[`LoginFailureMessage should match inactive user error message snapshot 1 `; +exports[`LoginFailureMessage should match internal server error message snapshot 1`] = ` +
    +
    + + We couldn't sign you in. + +
    + +
    +`; + exports[`LoginFailureMessage should match non compliant password error message snapshot 1`] = `