[client-app] fix circular errors and logging

Summary:
Errors weren't getting reported because of a circular reference causing
JSON.stringify to fail.

We can't add the whole API `response` object onto the error object,
because sometimes (like when the delta connection errors with a 401), the
`response` object is circular. We only use it to the extract the status
code anyway and it's a huge object. This diff excludes them from the
APIError objects.

We also have code to recover just in case JSON.stringify errors for some
other weird reason. It'll still attempt to report the error prefixed with
`Recovered Error`

Finally, we used to spit only the `error.stack` to the console. This meant
what we saw in the console didn't properly include the error messages
being incredibly not useful.

It's better to console.error the whole error object since that'll more
nicely display on the inspector console

Test Plan: manual

Reviewers: juan, spang, mark, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4154
This commit is contained in:
Evan Morikawa 2017-03-09 15:11:36 -05:00
parent 47f57a6c02
commit 3e895c74c9
3 changed files with 20 additions and 9 deletions

View file

@ -65,7 +65,22 @@ module.exports = ErrorLogger = (function() {
this._appendLog(error.stack) this._appendLog(error.stack)
if (extra) { this._appendLog(extra) } if (extra) { this._appendLog(extra) }
if (process.type === "renderer") { if (process.type === "renderer") {
var errorJSON = JSON.stringify(error); var errorJSON = "{}";
try {
errorJSON = JSON.stringify(error);
} catch (err) {
var recoveredError = new Error();
recoveredError.stack = error.stack;
recoveredError.message = `Recovered Error: ${error.message}`;
errorJSON = JSON.stringify(recoveredError)
}
var extraJSON;
try {
extraJSON = JSON.stringify(extra);
} catch (err) {
extraJSON = "{}";
}
/** /**
* We synchronously send all errors to the backend main process. * We synchronously send all errors to the backend main process.
@ -78,14 +93,12 @@ module.exports = ErrorLogger = (function() {
* This is a rare use of `sendSync` to ensure the command has made * This is a rare use of `sendSync` to ensure the command has made
* it before the window closes. * it before the window closes.
*/ */
ipcRenderer.sendSync("report-error", {errorJSON: errorJSON, extra: JSON.stringify(extra)}) ipcRenderer.sendSync("report-error", {errorJSON: errorJSON, extra: extraJSON})
} else { } else {
var nslog = require('nslog');
this._notifyExtensions("reportError", error, extra) this._notifyExtensions("reportError", error, extra)
nslog(error.stack)
} }
console.error(error, extra);
} }
ErrorLogger.prototype.openLogs = function() { ErrorLogger.prototype.openLogs = function() {

View file

@ -13,17 +13,16 @@ export class APIError extends Error {
this.name = "APIError"; this.name = "APIError";
this.error = error; this.error = error;
this.response = response;
this.body = body; this.body = body;
this.requestOptions = requestOptions; this.requestOptions = requestOptions;
this.statusCode = statusCode; this.statusCode = statusCode;
this.message = message; this.message = message;
if (this.statusCode == null) { if (this.statusCode == null) {
this.statusCode = this.response ? this.response.statusCode : null; this.statusCode = response ? response.statusCode : null;
} }
if (this.requestOptions == null) { if (this.requestOptions == null) {
this.requestOptions = this.response ? this.response.requestOptions : null; this.requestOptions = response ? response.requestOptions : null;
} }
this.stack = (new Error()).stack; this.stack = (new Error()).stack;

View file

@ -326,7 +326,6 @@ export default class NylasEnvConstructor {
this.emitter.emit('will-throw-error', event); this.emitter.emit('will-throw-error', event);
if (event.defaultPrevented) { return; } if (event.defaultPrevented) { return; }
console.error(error.stack, extra);
this.lastUncaughtError = error; this.lastUncaughtError = error;
extra.pluginIds = this._findPluginsFromError(error); extra.pluginIds = this._findPluginsFromError(error);