From d556e110cb2a30520999874b2ba760b83cf27ef2 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 26 Feb 2019 15:46:05 -0600 Subject: [PATCH] Include code in stringified errors This is done by returning the entire error stringified instead of just the message. This fixes the issue with the "save as" dialog. --- packages/ide/test/fs.test.ts | 5 ++- packages/protocol/src/browser/client.ts | 9 ++-- packages/protocol/src/common/util.ts | 21 ++++++--- packages/protocol/src/node/evaluate.ts | 9 ++-- packages/protocol/src/proto/node.proto | 8 +--- packages/protocol/src/proto/node_pb.d.ts | 16 ++----- packages/protocol/src/proto/node_pb.js | 57 +++++------------------- 7 files changed, 41 insertions(+), 84 deletions(-) diff --git a/packages/ide/test/fs.test.ts b/packages/ide/test/fs.test.ts index 856ae476d..e49afab64 100644 --- a/packages/ide/test/fs.test.ts +++ b/packages/ide/test/fs.test.ts @@ -496,8 +496,9 @@ describe("fs", () => { }); it("should fail to stat nonexistent file", async () => { - await expect(util.promisify(fs.stat)(tmpFile())) - .rejects.toThrow("ENOENT"); + const error = await util.promisify(fs.stat)(tmpFile()).catch((e) => e); + expect(error.message).toContain("ENOENT"); + expect(error.code).toBe("ENOENT"); }); }); diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 0e0960bd0..b20dea23f 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -98,7 +98,7 @@ export class Client { const eventsMsg = new EvalEventMessage(); eventsMsg.setId(doEval.id); eventsMsg.setEvent(event); - eventsMsg.setArgsList(args.map(stringify)); + eventsMsg.setArgsList(args.map((a) => stringify(a))); const clientMsg = new ClientMessage(); clientMsg.setEvalEvent(eventsMsg); this.connection.send(clientMsg.serializeBinary()); @@ -140,7 +140,7 @@ export class Client { const id = this.evalId++; newEval.setId(id); newEval.setActive(active); - newEval.setArgsList([a1, a2, a3, a4, a5, a6].map(stringify)); + newEval.setArgsList([a1, a2, a3, a4, a5, a6].map((a) => stringify(a))); newEval.setFunction(func.toString()); const clientMsg = new ClientMessage(); @@ -155,16 +155,15 @@ export class Client { const d1 = this.evalDoneEmitter.event((doneMsg) => { if (doneMsg.getId() === id) { - const resp = doneMsg.getResponse(); dispose(); - resolve(parse(resp)); + resolve(parse(doneMsg.getResponse())); } }); const d2 = this.evalFailedEmitter.event((failedMsg) => { if (failedMsg.getId() === id) { dispose(); - reject(new Error(failedMsg.getMessage())); + reject(parse(failedMsg.getResponse())); } }); }); diff --git a/packages/protocol/src/common/util.ts b/packages/protocol/src/common/util.ts index bc96ba9c9..9a2998b6d 100644 --- a/packages/protocol/src/common/util.ts +++ b/packages/protocol/src/common/util.ts @@ -25,17 +25,19 @@ export type IEncodingOptions = { export type IEncodingOptionsCallback = IEncodingOptions | ((err: NodeJS.ErrnoException, ...args: any[]) => void); /** - * Stringify an event argument. + * Stringify an event argument. isError is because although methods like + * `fs.stat` are supposed to throw Error objects, they currently throw regular + * objects when running tests through Jest. */ -export const stringify = (arg: any): string => { // tslint:disable-line no-any - if (arg instanceof Error) { +export const stringify = (arg: any, isError?: boolean): string => { // tslint:disable-line no-any + if (arg instanceof Error || isError) { // Errors don't stringify at all. They just become "{}". return JSON.stringify({ type: "Error", data: { message: arg.message, - name: arg.name, stack: arg.stack, + code: (arg as NodeJS.ErrnoException).code, }, }); } else if (arg instanceof Uint8Array) { @@ -74,7 +76,16 @@ export const parse = (arg: string): any => { // tslint:disable-line no-any // what happens to buffers and stringify them as regular objects. case "Error": if (result.data.message) { - return new Error(result.data.message); + const error = new Error(result.data.message); + // TODO: Can we set the stack? Doing so seems to make it into an + // "invalid object". + if (typeof result.data.code !== "undefined") { + (error as NodeJS.ErrnoException).code = result.data.code; + } + // tslint:disable-next-line no-any + (error as any).originalStack = result.data.stack; + + return error; } break; } diff --git a/packages/protocol/src/node/evaluate.ts b/packages/protocol/src/node/evaluate.ts index 3d66ccd90..58f07bafa 100644 --- a/packages/protocol/src/node/evaluate.ts +++ b/packages/protocol/src/node/evaluate.ts @@ -36,8 +36,7 @@ export const evaluate = (connection: SendableConnection, message: NewEvalMessage const sendException = (error: Error): void => { const evalFailed = new EvalFailedMessage(); evalFailed.setId(message.getId()); - evalFailed.setReason(EvalFailedMessage.Reason.EXCEPTION); - evalFailed.setMessage(error.toString() + " " + error.stack); + evalFailed.setResponse(stringify(error, true)); const serverMsg = new ServerMessage(); serverMsg.setEvalFailed(evalFailed); @@ -58,7 +57,7 @@ export const evaluate = (connection: SendableConnection, message: NewEvalMessage logger.trace(() => [ `${event}`, field("id", message.getId()), - field("args", args.map(stringify)), + field("args", args.map((a) => stringify(a))), ]); cb(...args); }); @@ -67,11 +66,11 @@ export const evaluate = (connection: SendableConnection, message: NewEvalMessage logger.trace(() => [ `emit ${event}`, field("id", message.getId()), - field("args", args.map(stringify)), + field("args", args.map((a) => stringify(a))), ]); const eventMsg = new EvalEventMessage(); eventMsg.setEvent(event); - eventMsg.setArgsList(args.map(stringify)); + eventMsg.setArgsList(args.map((a) => stringify(a))); eventMsg.setId(message.getId()); const serverMsg = new ServerMessage(); serverMsg.setEvalEvent(eventMsg); diff --git a/packages/protocol/src/proto/node.proto b/packages/protocol/src/proto/node.proto index 7af94becb..e71a04f0d 100644 --- a/packages/protocol/src/proto/node.proto +++ b/packages/protocol/src/proto/node.proto @@ -19,13 +19,7 @@ message EvalEventMessage { message EvalFailedMessage { uint64 id = 1; - enum Reason { - Timeout = 0; - Exception = 1; - Conflict = 2; - } - Reason reason = 2; - string message = 3; + string response = 2; } message EvalDoneMessage { diff --git a/packages/protocol/src/proto/node_pb.d.ts b/packages/protocol/src/proto/node_pb.d.ts index 3edcd2357..1b36e86b8 100644 --- a/packages/protocol/src/proto/node_pb.d.ts +++ b/packages/protocol/src/proto/node_pb.d.ts @@ -75,11 +75,8 @@ export class EvalFailedMessage extends jspb.Message { getId(): number; setId(value: number): void; - getReason(): EvalFailedMessage.Reason; - setReason(value: EvalFailedMessage.Reason): void; - - getMessage(): string; - setMessage(value: string): void; + getResponse(): string; + setResponse(value: string): void; serializeBinary(): Uint8Array; toObject(includeInstance?: boolean): EvalFailedMessage.AsObject; @@ -94,14 +91,7 @@ export class EvalFailedMessage extends jspb.Message { export namespace EvalFailedMessage { export type AsObject = { id: number, - reason: EvalFailedMessage.Reason, - message: string, - } - - export enum Reason { - TIMEOUT = 0, - EXCEPTION = 1, - CONFLICT = 2, + response: string, } } diff --git a/packages/protocol/src/proto/node_pb.js b/packages/protocol/src/proto/node_pb.js index ce4ccbe60..289eb9a9d 100644 --- a/packages/protocol/src/proto/node_pb.js +++ b/packages/protocol/src/proto/node_pb.js @@ -14,7 +14,6 @@ var global = Function('return this')(); goog.exportSymbol('proto.EvalDoneMessage', null, global); goog.exportSymbol('proto.EvalEventMessage', null, global); goog.exportSymbol('proto.EvalFailedMessage', null, global); -goog.exportSymbol('proto.EvalFailedMessage.Reason', null, global); goog.exportSymbol('proto.NewEvalMessage', null, global); /** @@ -554,8 +553,7 @@ proto.EvalFailedMessage.prototype.toObject = function(opt_includeInstance) { proto.EvalFailedMessage.toObject = function(includeInstance, msg) { var f, obj = { id: jspb.Message.getFieldWithDefault(msg, 1, 0), - reason: jspb.Message.getFieldWithDefault(msg, 2, 0), - message: jspb.Message.getFieldWithDefault(msg, 3, "") + response: jspb.Message.getFieldWithDefault(msg, 2, "") }; if (includeInstance) { @@ -597,12 +595,8 @@ proto.EvalFailedMessage.deserializeBinaryFromReader = function(msg, reader) { msg.setId(value); break; case 2: - var value = /** @type {!proto.EvalFailedMessage.Reason} */ (reader.readEnum()); - msg.setReason(value); - break; - case 3: var value = /** @type {string} */ (reader.readString()); - msg.setMessage(value); + msg.setResponse(value); break; default: reader.skipField(); @@ -640,32 +634,16 @@ proto.EvalFailedMessage.serializeBinaryToWriter = function(message, writer) { f ); } - f = message.getReason(); - if (f !== 0.0) { - writer.writeEnum( + f = message.getResponse(); + if (f.length > 0) { + writer.writeString( 2, f ); } - f = message.getMessage(); - if (f.length > 0) { - writer.writeString( - 3, - f - ); - } }; -/** - * @enum {number} - */ -proto.EvalFailedMessage.Reason = { - TIMEOUT: 0, - EXCEPTION: 1, - CONFLICT: 2 -}; - /** * optional uint64 id = 1; * @return {number} @@ -682,32 +660,17 @@ proto.EvalFailedMessage.prototype.setId = function(value) { /** - * optional Reason reason = 2; - * @return {!proto.EvalFailedMessage.Reason} - */ -proto.EvalFailedMessage.prototype.getReason = function() { - return /** @type {!proto.EvalFailedMessage.Reason} */ (jspb.Message.getFieldWithDefault(this, 2, 0)); -}; - - -/** @param {!proto.EvalFailedMessage.Reason} value */ -proto.EvalFailedMessage.prototype.setReason = function(value) { - jspb.Message.setProto3EnumField(this, 2, value); -}; - - -/** - * optional string message = 3; + * optional string response = 2; * @return {string} */ -proto.EvalFailedMessage.prototype.getMessage = function() { - return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 3, "")); +proto.EvalFailedMessage.prototype.getResponse = function() { + return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 2, "")); }; /** @param {string} value */ -proto.EvalFailedMessage.prototype.setMessage = function(value) { - jspb.Message.setProto3StringField(this, 3, value); +proto.EvalFailedMessage.prototype.setResponse = function(value) { + jspb.Message.setProto3StringField(this, 2, value); };