Skip to content

Commit e109fdd

Browse files
authored
Prompt the user to refresh the page if their session expires (#2241)
1 parent 3c2199c commit e109fdd

File tree

10 files changed

+288
-90
lines changed

10 files changed

+288
-90
lines changed

src/core/constants.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,3 +174,11 @@ export const GLOBAL_EVENT_STATUS_MAP = {
174174
// The events here are set directly on mozAddonManager
175175
// they will be fired by addons and themes.
176176
export const GLOBAL_EVENTS = Object.keys(GLOBAL_EVENT_STATUS_MAP);
177+
178+
// Generic error codes.
179+
export const ERROR_UNKNOWN = 'ERROR_UNKNOWN';
180+
// API error codes. These values match the error codes defined here:
181+
// http://addons-server.readthedocs.io/en/latest/topics/api/overview.html#unauthorized-and-permission-denied
182+
export const API_ERROR_DECODING_SIGNATURE = 'ERROR_DECODING_SIGNATURE';
183+
export const API_ERROR_INVALID_HEADER = 'ERROR_INVALID_HEADER';
184+
export const API_ERROR_SIGNATURE_EXPIRED = 'ERROR_SIGNATURE_EXPIRED';

src/core/css/ErrorHandler.scss

Lines changed: 0 additions & 15 deletions
This file was deleted.

src/core/css/inc/vars.scss

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ $sub-text-color: #6a6a6a;
1111
$form-border-color: $primary-font-color;
1212
$error-text-color: $text-color-message;
1313
$error-background-color: #f03e3e;
14+
$error-button-background-color: $error-text-color;
15+
$error-button-text-color: $error-background-color;
1416

1517
$button-background-color: #0095dd;
1618

src/core/errorHandler.js

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import { connect } from 'react-redux';
44

55
import { clearError, setError } from 'core/actions/errors';
66
import log from 'core/logger';
7-
8-
import 'core/css/ErrorHandler.scss';
7+
import ErrorList from 'ui/components/ErrorList';
98

109
function generateHandlerId({ name = '' } = {}) {
1110
return `${name}-${Math.random().toString(36).substr(2, 9)}`;
@@ -35,23 +34,8 @@ export class ErrorHandler {
3534
}
3635

3736
renderError() {
38-
return (
39-
<ul className="ErrorHandler-list">
40-
{this.capturedError.messages.map(
41-
(msg) => {
42-
let msgString = msg;
43-
if (typeof msgString === 'object') {
44-
// This is an unlikely scenario where an API response
45-
// contains nested objects within objects. If this
46-
// happens in real life let's make it prettier.
47-
// Until then, let's just prevent a stack trace.
48-
msgString = JSON.stringify(msg);
49-
}
50-
return <li className="ErrorHandler-list-item">{msgString}</li>;
51-
}
52-
)}
53-
</ul>
54-
);
37+
const { code, messages } = this.capturedError;
38+
return <ErrorList messages={messages} code={code} />;
5539
}
5640

5741
handle(error) {

src/core/reducers/errors.js

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { CLEAR_ERROR, SET_ERROR } from 'core/constants';
1+
import { CLEAR_ERROR, ERROR_UNKNOWN, SET_ERROR } from 'core/constants';
22
import log from 'core/logger';
3-
import { gettext } from 'core/utils';
43

54
/*
65
* This inspects an error object and returns an array of messages from it.
@@ -12,41 +11,45 @@ import { gettext } from 'core/utils';
1211
* http://addons-server.readthedocs.io/en/latest/topics/api/overview.html#responses
1312
*/
1413
function getMessagesFromError(error) {
15-
let messages = [gettext('An unexpected error occurred')];
14+
let errorData = {
15+
code: ERROR_UNKNOWN,
16+
messages: [],
17+
};
1618
log.info('Extracting messages from error object:', error);
1719

1820
if (error && error.response && error.response.data) {
19-
const apiMessages = [];
20-
2121
Object.keys(error.response.data).forEach((key) => {
2222
const val = error.response.data[key];
23+
if (key === 'code') {
24+
errorData = { ...errorData, code: val };
25+
return;
26+
}
2327
if (Array.isArray(val)) {
2428
// Most API reponse errors will consist of a key (which could be a
2529
// form field) and an array of localized messages.
2630
// More info: http://addons-server.readthedocs.io/en/latest/topics/api/overview.html#bad-requests
2731
val.forEach((msg) => {
2832
if (key === 'non_field_errors') {
2933
// Add a generic error not related to a specific field.
30-
apiMessages.push(msg);
34+
errorData.messages.push(msg);
3135
} else {
3236
// Add field specific error message.
3337
// The field is not localized but we need to show it as a hint.
34-
apiMessages.push(`${key}: ${msg}`);
38+
errorData.messages.push(`${key}: ${msg}`);
3539
}
3640
});
3741
} else {
3842
// This is most likely not a form field error so just show the message.
39-
apiMessages.push(val);
43+
errorData.messages.push(val);
4044
}
4145
});
46+
}
4247

43-
if (apiMessages.length) {
44-
messages = apiMessages;
45-
} else {
46-
log.warn('API error response did not contain any messages', error);
47-
}
48+
if (!errorData.messages.length) {
49+
log.warn('Error object did not contain any messages', error);
4850
}
49-
return messages;
51+
52+
return errorData;
5053
}
5154

5255
export const initialState = {};
@@ -58,13 +61,14 @@ export default function errors(state = initialState, action) {
5861
...state,
5962
[action.payload.id]: null,
6063
};
61-
case SET_ERROR:
64+
case SET_ERROR: {
65+
const { code, messages } =
66+
getMessagesFromError(action.payload.error);
6267
return {
6368
...state,
64-
[action.payload.id]: {
65-
messages: getMessagesFromError(action.payload.error),
66-
},
69+
[action.payload.id]: { code, messages },
6770
};
71+
}
6872
default:
6973
return state;
7074
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/* global window */
2+
import classNames from 'classnames';
3+
import React, { PropTypes } from 'react';
4+
import { compose } from 'redux';
5+
6+
import { API_ERROR_SIGNATURE_EXPIRED } from 'core/constants';
7+
import log from 'core/logger';
8+
import translate from 'core/i18n/translate';
9+
import Button from 'ui/components/Button';
10+
11+
import './styles.scss';
12+
13+
14+
class ErrorList extends React.Component {
15+
static propTypes = {
16+
_window: PropTypes.object,
17+
code: PropTypes.string,
18+
className: PropTypes.string,
19+
i18n: PropTypes.object.isRequired,
20+
messages: PropTypes.array.isRequired,
21+
}
22+
23+
static defaultProps = {
24+
_window: typeof window !== 'undefined' ? window : {},
25+
};
26+
27+
render() {
28+
const { _window, code, className, i18n, messages } = this.props;
29+
const items = [];
30+
31+
messages.forEach((messageItem) => {
32+
let message = messageItem;
33+
if (typeof message === 'object') {
34+
// This handles an unlikely scenario where an API error response
35+
// contains nested objects within objects. If this happens in real
36+
// life let's fix it or make the display prettier.
37+
// Until then, let's just prevent it from triggering an exception.
38+
message = JSON.stringify(message);
39+
}
40+
if (code === API_ERROR_SIGNATURE_EXPIRED) {
41+
// This API error describes exactly what happened but that isn't
42+
// very helpful for AMO users. Let's help them figure it out.
43+
log.debug(`Detected ${code}, replacing API translation: ${message}`);
44+
message = i18n.gettext('Your session has expired');
45+
}
46+
items.push(message);
47+
});
48+
49+
if (!items.length) {
50+
log.debug(`No messages were passed to ErrorList, code: ${code}`);
51+
items.push(i18n.gettext('An unexpected error occurred'));
52+
}
53+
54+
if (code === API_ERROR_SIGNATURE_EXPIRED) {
55+
items.push(
56+
<Button onClick={() => _window.location.reload()}>
57+
{i18n.gettext('Reload To Continue')}
58+
</Button>
59+
);
60+
}
61+
62+
return (
63+
<ul className={classNames('ErrorList', className)}>
64+
{items.map((item) => <li className="ErrorList-item">{item}</li>)}
65+
</ul>
66+
);
67+
}
68+
}
69+
70+
export default compose(
71+
translate({ withRef: true }),
72+
)(ErrorList);
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
@import "~core/css/inc/vars";
2+
3+
.ErrorList {
4+
text-align: center;
5+
border-radius: $border-radius-default;
6+
background-color: $error-background-color;
7+
color: $error-text-color;
8+
padding: 10px;
9+
margin: 0;
10+
margin-bottom: 10px;
11+
}
12+
13+
.ErrorList-item {
14+
list-style: none;
15+
margin-bottom: 8px;
16+
17+
.Button {
18+
color: $error-button-text-color;
19+
background-color: $error-button-background-color;
20+
border-radius: $border-radius-s;
21+
font-size: $font-size-s;
22+
}
23+
24+
&:last-child {
25+
margin-bottom: 0;
26+
}
27+
}

tests/client/core/reducers/test_errors.js

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createApiError } from 'core/api/index';
22
import { clearError, setError } from 'core/actions/errors';
3+
import { API_ERROR_SIGNATURE_EXPIRED, ERROR_UNKNOWN } from 'core/constants';
34
import errors, { initialState } from 'core/reducers/errors';
45

56
export function createFakeApiError({ fieldErrors = {}, nonFieldErrors } = {}) {
@@ -23,15 +24,6 @@ describe('errors reducer', () => {
2324
assert.deepEqual(errors(undefined, { type: 'UNRELATED' }), initialState);
2425
});
2526

26-
it('stores a simple error', () => {
27-
const error = new Error('some message');
28-
const action = setError({ id: 'some-id', error });
29-
const state = errors(undefined, action);
30-
assert.deepEqual(state[action.payload.id], {
31-
messages: ['An unexpected error occurred'],
32-
});
33-
});
34-
3527
it('handles API object responses', () => {
3628
const message = 'Authentication credentials were not provided.';
3729
const error = createApiError({
@@ -41,7 +33,10 @@ describe('errors reducer', () => {
4133
});
4234
const action = setError({ id: 'some-id', error });
4335
const state = errors(undefined, action);
44-
assert.deepEqual(state[action.payload.id], { messages: [message] });
36+
assert.deepEqual(state[action.payload.id], {
37+
code: ERROR_UNKNOWN,
38+
messages: [message],
39+
});
4540
});
4641

4742
it('preserves existing errors', () => {
@@ -85,11 +80,13 @@ describe('errors reducer', () => {
8580
assert.equal(state.action2.messages[0], 'action2');
8681
});
8782

88-
it('creates a default error message', () => {
83+
it('stores a generic error', () => {
8984
const action = setError({ id: 'action1', error: new Error('any message') });
9085
const state = errors(undefined, action);
91-
assert.deepEqual(state[action.payload.id].messages,
92-
['An unexpected error occurred']);
86+
assert.deepEqual(state[action.payload.id], {
87+
code: ERROR_UNKNOWN,
88+
messages: [],
89+
});
9390
});
9491

9592
it('gets non_field_errors from API error response', () => {
@@ -123,12 +120,41 @@ describe('errors reducer', () => {
123120
assert.include(messages, 'password: sorry, it cannot be 1234');
124121
});
125122

126-
it('handles API responses without any messages', () => {
123+
it('stores API responses when they do not have messages', () => {
127124
// This API error has no messages (hopefully this won't ever happen).
128125
const action = setError({ id: 'some-id', error: createFakeApiError() });
129126
const state = errors(undefined, action);
130127
assert.deepEqual(state[action.payload.id], {
131-
messages: ['An unexpected error occurred'],
128+
code: ERROR_UNKNOWN,
129+
messages: [],
130+
});
131+
});
132+
133+
it('adds an error code', () => {
134+
const error = createApiError({
135+
response: { status: 401 },
136+
apiURL: 'https://some/api/endpoint',
137+
jsonResponse: {
138+
code: API_ERROR_SIGNATURE_EXPIRED,
139+
detail: 'Any message about an expired signature.',
140+
},
132141
});
142+
const action = setError({ id: 'some-id', error });
143+
const state = errors(undefined, action);
144+
assert.equal(state[action.payload.id].code, API_ERROR_SIGNATURE_EXPIRED);
145+
});
146+
147+
it('does not turn an error code into a message', () => {
148+
const error = createApiError({
149+
response: { status: 401 },
150+
apiURL: 'https://some/api/endpoint',
151+
jsonResponse: {
152+
code: API_ERROR_SIGNATURE_EXPIRED,
153+
detail: 'Some message.',
154+
},
155+
});
156+
const action = setError({ id: 'some-id', error });
157+
const state = errors(undefined, action);
158+
assert.deepEqual(state[action.payload.id].messages, ['Some message.']);
133159
});
134160
});

0 commit comments

Comments
 (0)