Add more info to server errors

Also removes the hardcoded 'Server error' text in favor of 'Error', as
sometimes PG may be misconfigured (i.e. the error is actually due to the
client).

Bug: Issue 9773
Change-Id: I75759c9208af30ca4b8cf4606434f8bcdcdc9ece
This commit is contained in:
Kasper Nilsson
2018-10-05 13:25:09 -07:00
parent ad5c4bb899
commit 3530f87db0
3 changed files with 56 additions and 21 deletions

View File

@@ -91,22 +91,36 @@
},
_handleServerError(e) {
Promise.all([
e.detail.response.text(), this._getLoggedIn(),
]).then(values => {
const text = values[0];
const loggedIn = values[1];
if (e.detail.response.status === 403 &&
loggedIn &&
text === AUTHENTICATION_REQUIRED) {
// The app was logged at one point and is now getting auth errors.
// This indicates the auth token is no longer valid.
this._handleAuthError();
} else if (!this._shouldSuppressError(text)) {
this._showErrorDialog('Server error: ' + text);
}
console.error(text);
});
const {request, response} = e.detail;
Promise.all([response.text(), this._getLoggedIn()])
.then(([errorText, loggedIn]) => {
const url = request && (request.anonymizedUrl || request.url);
const {status, statusText} = response;
if (response.status === 403 &&
loggedIn &&
errorText === AUTHENTICATION_REQUIRED) {
// The app was logged at one point and is now getting auth errors.
// This indicates the auth token is no longer valid.
this._handleAuthError();
} else if (!this._shouldSuppressError(errorText)) {
this._showErrorDialog(this._constructServerErrorMsg({
status,
statusText,
errorText,
url,
}));
}
console.error(errorText);
});
},
_constructServerErrorMsg({errorText, status, statusText, url}) {
let err = `Error ${status}`;
if (statusText) { err += ` (${statusText})`; }
if (errorText || url) { err += ': '; }
if (errorText) { err += errorText; }
if (url) { err += `\nEndpoint: ${url}`; }
return err;
},
_handleShowAlert(e) {

View File

@@ -86,7 +86,7 @@ limitations under the License.
'Log in is required to perform that action.', 'Log in.'));
});
test('show normal server error', done => {
test('show normal Error', done => {
const showErrorStub = sandbox.stub(element, '_showErrorDialog');
const textSpy = sandbox.spy(() => { return Promise.resolve('ZOMG'); });
element.fire('server-error', {response: {status: 500, text: textSpy}});
@@ -98,11 +98,32 @@ limitations under the License.
]).then(() => {
assert.isTrue(showErrorStub.calledOnce);
assert.isTrue(showErrorStub.lastCall.calledWithExactly(
'Server error: ZOMG'));
'Error 500: ZOMG'));
done();
});
});
test('_constructServerErrorMsg', () => {
const errorText = 'change conflicts';
const status = 409;
const statusText = 'Conflict';
const url = '/my/test/url';
assert.equal(element._constructServerErrorMsg({status}),
'Error 409');
assert.equal(element._constructServerErrorMsg({status, url}),
'Error 409: \nEndpoint: /my/test/url');
assert.equal(element._constructServerErrorMsg({status, statusText, url}),
'Error 409 (Conflict): \nEndpoint: /my/test/url');
assert.equal(element._constructServerErrorMsg({
status,
statusText,
errorText,
url,
}), 'Error 409 (Conflict): change conflicts' +
'\nEndpoint: /my/test/url');
});
test('suppress TOO_MANY_FILES error', done => {
const showAlertStub = sandbox.stub(element, '_showAlert');
const textSpy = sandbox.spy(() => {

View File

@@ -298,7 +298,7 @@
req.errFn.call(null, response);
return;
}
this.fire('server-error', {response});
this.fire('server-error', {request: req, response});
return;
}
return response && this.getResponseObject(response);
@@ -1307,7 +1307,7 @@
if (opt_errFn) {
opt_errFn.call(null, response);
} else {
this.fire('server-error', {response});
this.fire('server-error', {request: req, response});
}
return;
}
@@ -2086,7 +2086,7 @@
if (req.errFn) {
return req.errFn.call(undefined, response);
}
this.fire('server-error', {response});
this.fire('server-error', {request: fetchReq, response});
}
return response;
}).catch(err => {