From 2ee9c61561c45d0893d186395863503f3369b784 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Tue, 6 Jun 2017 13:32:06 -0700 Subject: [PATCH 01/11] error-reporting: Fix `report()` silently failing If the `report()` method is called with a number, an associated error never appears in the error reporting console, and no log message is printed to the user to indicate why the error has not appeared. --- .../src/classes/custom-stack-trace.js | 117 --------- .../src/error-extractors/error.js | 49 ---- .../src/error-extractors/object.js | 80 ------ .../src/error-handlers/error.js | 46 ---- .../src/error-handlers/number.js | 45 ---- .../src/error-handlers/object.js | 45 ---- .../src/error-handlers/string.js | 47 ---- .../src/error-handlers/unknown.js | 37 --- packages/error-reporting/src/error-router.js | 196 ++++++++++++--- .../test/unit/classes/custom-stack-trace.js | 77 ------ .../test/unit/error-extractors/error.js | 107 -------- .../test/unit/error-extractors/object.js | 108 -------- .../test/unit/error-handlers/error.js | 56 ----- .../test/unit/error-handlers/number.js | 44 ---- .../test/unit/error-handlers/object.js | 44 ---- .../test/unit/error-handlers/string.js | 44 ---- .../test/unit/error-handlers/unknown.js | 47 ---- .../error-reporting/test/unit/error-router.js | 236 ++++++++++++++++++ 18 files changed, 401 insertions(+), 1024 deletions(-) delete mode 100644 packages/error-reporting/src/classes/custom-stack-trace.js delete mode 100644 packages/error-reporting/src/error-extractors/error.js delete mode 100644 packages/error-reporting/src/error-extractors/object.js delete mode 100644 packages/error-reporting/src/error-handlers/error.js delete mode 100644 packages/error-reporting/src/error-handlers/number.js delete mode 100644 packages/error-reporting/src/error-handlers/object.js delete mode 100644 packages/error-reporting/src/error-handlers/string.js delete mode 100644 packages/error-reporting/src/error-handlers/unknown.js delete mode 100644 packages/error-reporting/test/unit/classes/custom-stack-trace.js delete mode 100644 packages/error-reporting/test/unit/error-extractors/error.js delete mode 100644 packages/error-reporting/test/unit/error-extractors/object.js delete mode 100644 packages/error-reporting/test/unit/error-handlers/error.js delete mode 100644 packages/error-reporting/test/unit/error-handlers/number.js delete mode 100644 packages/error-reporting/test/unit/error-handlers/object.js delete mode 100644 packages/error-reporting/test/unit/error-handlers/string.js delete mode 100644 packages/error-reporting/test/unit/error-handlers/unknown.js create mode 100644 packages/error-reporting/test/unit/error-router.js diff --git a/packages/error-reporting/src/classes/custom-stack-trace.js b/packages/error-reporting/src/classes/custom-stack-trace.js deleted file mode 100644 index 7e9d3ea8eba..00000000000 --- a/packages/error-reporting/src/classes/custom-stack-trace.js +++ /dev/null @@ -1,117 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var is = require('is'); -var isString = is.string; -var isNumber = is.number; -var isFunction = is.fn; - -/** - * A function which is used as a default substitute to handle cases where an - * Error's call site array was not properly generated or its length was not - * greater than 0. This function aims to produce a basic notification that - * CallSite extraction failed. - * @function stubbedNoOp - * @returns {String} - A JSON object serving as a notification with one - * property: error whose value is a string describing that a stack trace could - * not be captured. - */ -function stubbedNoOp() { - - return JSON.stringify({error: 'Unable to capture stack trace information'}); -} - -/** - * The constuctor for CustomStackTrace takes no arugments and is solely meant - * to instantiate properties on the instance. Each property should be externally - * set using the corresponding set function. - * @class CustomStackTrace - * @classdesc CustomStackTrace is a class which is meant to store and control- - * for top-frame values on a stack trace dump. This class can be stringified - * for representation in JSON. - * @property {String} filePath - The file path of CallSite object - * @property {Number} lineNumber - The line number of the CallSite object - * @property {String} functionName - The function name of the CallSite object - * @property {Function} stringifyStucturedCallList - A function which is meant - * to produce the JSON representation of the full stack trace. - */ -function CustomStackTrace() { - - this.filePath = ''; - this.lineNumber = 0; - this.functionName = ''; - this.stringifyStucturedCallList = stubbedNoOp; -} - -/** - * Sets the filePath property on the instance - * @function setFilePath - * @chainable - * @param {String} filePath - the file path of the CallSite object - * @returns {this} - returns the instance for chaining - */ -CustomStackTrace.prototype.setFilePath = function(filePath) { - - this.filePath = isString(filePath) ? filePath : ''; - - return this; -}; - -/** - * Sets the lineNumber property on the instance - * @function setLineNumber - * @chainable - * @param {Number} lineNumber - the line number of the CallSite object - * @returns {this} - returns the instance for chaining - */ -CustomStackTrace.prototype.setLineNumber = function(lineNumber) { - - this.lineNumber = isNumber(lineNumber) ? lineNumber : 0; - - return this; -}; - -/** - * Sets the functionName property on the instance - * @function setFunctionName - * @chainable - * @param {String} functionName - the function name of the CallSite object - * @returns {this} - returns the instance for chaining - */ -CustomStackTrace.prototype.setFunctionName = function(functionName) { - - this.functionName = isString(functionName) ? functionName : ''; - - return this; -}; - -/** - * Sets the stringifyStucturedCallList on the instance - * @function setStringifyStructuredCallList - * @chainable - * @param {Function} op - the function to produce the JSON representation of - * the full stack trace - * @returns {this} - returns the instance for chaining - */ -CustomStackTrace.prototype.setStringifyStructuredCallList = function(op) { - - this.stringifyStucturedCallList = isFunction(op) ? op : stubbedNoOp; - - return this; -}; - -module.exports = CustomStackTrace; diff --git a/packages/error-reporting/src/error-extractors/error.js b/packages/error-reporting/src/error-extractors/error.js deleted file mode 100644 index e2abe83ec5d..00000000000 --- a/packages/error-reporting/src/error-extractors/error.js +++ /dev/null @@ -1,49 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var has = require('lodash.has'); -var is = require('is'); -var isObject = is.object; - -/** - * Extracts error information from an instance of the Error class and marshals - * that information into the provided instance of error message. This function - * will check before accessing any part of the error for propety presence but - * will not check the types of these property values that is instead work that - * is allocated to the error message instance itself. - * @param {Error} err - the error instance - * @param {ErrorMessage} errorMessage - the error message instance to have the - * error information marshaled into - * @returns {Undefined} - does not return anything - */ -function extractFromErrorClass(err, errorMessage) { - - errorMessage.setMessage(err.stack); - - if (has(err, 'user')) { - - errorMessage.setUser(err.user); - } - - if (has(err, 'serviceContext') && isObject(err.serviceContext)) { - - errorMessage.setServiceContext(err.serviceContext.service, - err.serviceContext.version); - } -} - -module.exports = extractFromErrorClass; diff --git a/packages/error-reporting/src/error-extractors/object.js b/packages/error-reporting/src/error-extractors/object.js deleted file mode 100644 index f1b155177e4..00000000000 --- a/packages/error-reporting/src/error-extractors/object.js +++ /dev/null @@ -1,80 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var has = require('lodash.has'); -var is = require('is'); -var isObject = is.object; - -/** - * Attempts to extract error information given an object as the input for the - * error. This function will check presence of each property before attempting - * to access the given property on the object but will not check for type - * compliance as that is allocated to the instance of the error message itself. - * @function extractFromObject - * @param {Object} err - the Object given as the content of the error - * @param {String} [err.message] - the error message - * @param {String} [err.user] - the user the error occurred for - * @param {String} [err.filePath] - the file path and file where the error - * occurred at - * @param {Number} [err.lineNumber] - the line number where the error occurred - * at - * @param {String} [err.functionName] - the function where the error occurred at - * @param {Object} [err.serviceContext] - the service context object of the - * error - * @param {String} [err.serviceContext.service] - the service the error occurred - * on - * @param {String} [err.serviceContext.version] - the version of the application - * that the error occurred on - * @param {ErrorMessage} errorMessage - the error message instance to marshal - * error information into - * @returns {Undefined} - does not return anything - */ -function extractFromObject(err, errorMessage) { - - if (has(err, 'message')) { - - errorMessage.setMessage(err.message); - } - - if (has(err, 'user')) { - - errorMessage.setUser(err.user); - } - - if (has(err, 'filePath')) { - - errorMessage.setFilePath(err.filePath); - } - - if (has(err, 'lineNumber')) { - - errorMessage.setLineNumber(err.lineNumber); - } - - if (has(err, 'functionName')) { - - errorMessage.setFunctionName(err.functionName); - } - - if (has(err, 'serviceContext') && isObject(err.serviceContext)) { - - errorMessage.setServiceContext(err.serviceContext.service, - err.serviceContext.version); - } -} - -module.exports = extractFromObject; diff --git a/packages/error-reporting/src/error-handlers/error.js b/packages/error-reporting/src/error-handlers/error.js deleted file mode 100644 index a3ca5a938e6..00000000000 --- a/packages/error-reporting/src/error-handlers/error.js +++ /dev/null @@ -1,46 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var handleUnknownAsError = require('./unknown.js'); -var extractFromErrorClass = require('../error-extractors/error.js'); - -/** - * Handles routing and validation for parsing an errorMessage that was - * flagged as an instance of the Error class. This function does not - * discriminate against regular objects, checking only to see if the err - * parameter is itself a basic object and has the function property - * hasOwnProperty. Given that the input passes this basic test the input - * will undergo extraction by the extractFromErrorClass function, otherwise - * it will be treated and processed as an unknown. - * @function handleErrorClassError - * @param {Error} err - the error instance to extract information from - * @param {ErrorMessage} errorMessage - the error message to marshal error - * information into. - * @returns {Undefined} - does not return anything - */ -function handleErrorClassError(err, errorMessage) { - - if (err instanceof Error) { - - extractFromErrorClass(err, errorMessage); - } else { - - handleUnknownAsError(err, errorMessage); - } -} - -module.exports = handleErrorClassError; diff --git a/packages/error-reporting/src/error-handlers/number.js b/packages/error-reporting/src/error-handlers/number.js deleted file mode 100644 index ebec109028e..00000000000 --- a/packages/error-reporting/src/error-handlers/number.js +++ /dev/null @@ -1,45 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var is = require('is'); -var isNumber = is.number; -var isFunction = is.function; - -/** - * Handles routing and validation for parsing an error which has been indicated - * to be of type Number. This handler will manufacture a new Error to create - * a stack-trace for submission to the Error API and will attempt to caste the - * given number to a string for submission to the Error API. - * @function handleNumberAsError - * @param {Number} err - the number submitted as content for the error message - * @param {ErrorMessage} errorMessage - the error messag instance to marshall - * error information into. - * @returns {Undefined} - does not return anything - */ -function handleNumberAsError(err, errorMessage) { - var fauxError = new Error(); - var errChecked = fauxError.stack; - - if (isNumber(err) && isFunction(err.toString)) { - - errChecked = err.toString(); - } - - errorMessage.setMessage(errChecked); -} - -module.exports = handleNumberAsError; diff --git a/packages/error-reporting/src/error-handlers/object.js b/packages/error-reporting/src/error-handlers/object.js deleted file mode 100644 index 2b3c11a13c2..00000000000 --- a/packages/error-reporting/src/error-handlers/object.js +++ /dev/null @@ -1,45 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var is = require('is'); -var isObject = is.object; -var extractFromObject = require('../error-extractors/object.js'); -var handleUnknownAsError = require('./unknown.js'); - -/** - * Handles routing and validation for parsing an error that has been indicated - * to be of type object. If the value submitted for err passes a basic check - * for being of type Object than the input will extracted as such, otherwise the - * input will be treated as unknown. - * @function handleObjectAsError - * @param {Object} err - the error information object to extract from - * @param {ErrorMessage} errorMessage - the error message instance to marshal - * error information into - * @returns {Undefined} - does not return anything - */ -function handleObjectAsError(err, errorMessage) { - - if (isObject(err)) { - - extractFromObject(err, errorMessage); - } else { - - handleUnknownAsError(err, errorMessage); - } -} - -module.exports = handleObjectAsError; diff --git a/packages/error-reporting/src/error-handlers/string.js b/packages/error-reporting/src/error-handlers/string.js deleted file mode 100644 index 8e2e0fb816f..00000000000 --- a/packages/error-reporting/src/error-handlers/string.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; -var isString = require('is').string; - -/** - * Handles validation of an error which has been indicated to be of type String. - * This function will create a new instance of the Error class to produce a - * stack trace for submission to the API and check to confirm that the given - * value is of type string. - * @function handleStringAsError - * @param {String} err - the String indicated as the content of the error - * @param {ErrorMessage} errorMessage - the error message instance to marshal - * error information into. - * @returns {Undefined} - does not return anything - */ -function handleStringAsError(err, errorMessage) { - var fauxError = new Error(); - var fullStack = fauxError.stack.split('\n'); - var cleanedStack = fullStack.slice(0, 1).concat(fullStack.slice(4)); - var errChecked = ''; - - if (isString(err)) { - // Replace the generic error message with the user-provided string - cleanedStack[0] = err; - } - - errChecked = cleanedStack.join('\n'); - - errorMessage.setMessage(errChecked); -} - -module.exports = handleStringAsError; diff --git a/packages/error-reporting/src/error-handlers/unknown.js b/packages/error-reporting/src/error-handlers/unknown.js deleted file mode 100644 index 6243f184bc5..00000000000 --- a/packages/error-reporting/src/error-handlers/unknown.js +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -/** - * Handles unknown/unsupported input as the content of the error message. Since - * the problem-space is not defined for this path the library only attempts to - * manufacture a stack trace for submission to the API and discards the input - * that was given as the error content. - * @function handleUnknownAsError - * @param {Any} err - the unknown/unsupported input indicated as the content of - * the error. - * @param {ErrorMessage} errorMessage - the error message instance to marshal - * error information into. - * @returns {Undefined} - does not return anything - */ -function handleUnknownAsError(err, errorMessage) { - var fauxError = new Error(); - - errorMessage.setMessage(fauxError.stack); -} - -module.exports = handleUnknownAsError; diff --git a/packages/error-reporting/src/error-router.js b/packages/error-reporting/src/error-router.js index d8dbcf5ab8e..78ef83d45f1 100644 --- a/packages/error-reporting/src/error-router.js +++ b/packages/error-reporting/src/error-router.js @@ -15,49 +15,183 @@ */ 'use strict'; -var handleErrorClassError = require('./error-handlers/error.js'); -var handleObjectAsError = require('./error-handlers/object.js'); -var handleStringAsError = require('./error-handlers/string.js'); -var handleNumberAsError = require('./error-handlers/number.js'); -var handleUnknownAsError = require('./error-handlers/unknown.js'); +var has = require('lodash.has'); +var is = require('is'); +var isObject = is.object; +var isString = is.string; +var isNumber = is.number; +var isFunction = is.function; /** - * The Error handler router is responsible for taking an error of some type and - * and Error message container, analyzing the type of the error and routing it - * to the proper handler so that the error information can be marshaled into the - * the error message container. + * The Error handler router is responsible for taking an object of some type and + * and Error message container, analyzing the type of the object and marshalling + * the object's information into the error message container. * @function errorHandlerRouter - * @param {Any} err - the error information to extract from - * @param {ErrorMessage} em - an instance of ErrorMessage to marshal error + * @param {Any} ob - the object information to extract from + * @param {ErrorMessage} em - an instance of ErrorMessage to marshal object * information into * @returns {Undefined} - does not return a value */ -function errorHandlerRouter(err, em) { +function errorHandlerRouter(ob, em) { + if (ob === null || ob === undefined) { + populateFromUnknown(ob, em); + } + else if (ob instanceof Error) { + populateFromError(ob, em); + } + else if (typeof ob === 'object' && isObject(ob)) { + populateFromObject(ob, em); + } + else if (typeof ob === 'string' && isString(ob)) { + populateFromString(ob, em); + } + else if (typeof ob === 'number') { + populateFromNumber(ob, em); + } + else { + populateFromUnknown(ob, em); + } + + return em; +} + +/** + * Extracts error information from an instance of the Error class and marshals + * that information into the provided instance of error message. This function + * will check before accessing any part of the error for propety presence but + * will not check the types of these property values that is instead work that + * is allocated to the error message instance itself. + * @param {Error} err - the error instance + * @param {ErrorMessage} errorMessage - the error message instance to have the + * error information marshaled into + * @returns {Undefined} - does not return anything + */ +function populateFromError(err, errorMessage) { + errorMessage.setMessage(err.stack); + + if (has(err, 'user')) { + errorMessage.setUser(err.user); + } + + if (has(err, 'serviceContext') && isObject(err.serviceContext)) { + errorMessage.setServiceContext(err.serviceContext.service, + err.serviceContext.version); + } +} + +/** + * Attempts to extract error information given an object as the input for the + * error. This function will check presence of each property before attempting + * to access the given property on the object but will not check for type + * compliance as that is allocated to the instance of the error message itself. + * @function extractFromObject + * @param {Object} err - the Object given as the content of the error + * @param {String} [err.message] - the error message + * @param {String} [err.user] - the user the error occurred for + * @param {String} [err.filePath] - the file path and file where the error + * occurred at + * @param {Number} [err.lineNumber] - the line number where the error occurred + * at + * @param {String} [err.functionName] - the function where the error occurred at + * @param {Object} [err.serviceContext] - the service context object of the + * error + * @param {String} [err.serviceContext.service] - the service the error occurred + * on + * @param {String} [err.serviceContext.version] - the version of the application + * that the error occurred on + * @param {ErrorMessage} errorMessage - the error message instance to marshal + * error information into + * @returns {Undefined} - does not return anything + */ +function populateFromObject(ob, errorMessage) { + if (has(ob, 'message')) { + errorMessage.setMessage(ob.message); + } + + if (has(ob, 'user')) { + errorMessage.setUser(ob.user); + } - if (err instanceof Error) { + if (has(ob, 'filePath')) { + errorMessage.setFilePath(ob.filePath); + } - handleErrorClassError(err, em); + if (has(ob, 'lineNumber')) { + errorMessage.setLineNumber(ob.lineNumber); + } - return; + if (has(ob, 'functionName')) { + errorMessage.setFunctionName(ob.functionName); } - switch (typeof err) { - case 'object': { - handleObjectAsError(err, em); - break; - } - case 'string': { - handleStringAsError(err, em); - break; - } - case 'number': { - handleNumberAsError(err, em); - break; - } - default: { - handleUnknownAsError(err, em); - } + if (has(ob, 'serviceContext') && isObject(ob.serviceContext)) { + errorMessage.setServiceContext(ob.serviceContext.service, + ob.serviceContext.version); } } +/** + * Handles validation of an error which has been indicated to be of type String. + * This function will create a new instance of the Error class to produce a + * stack trace for submission to the API and check to confirm that the given + * value is of type string. + * @function handleStringAsError + * @param {String} err - the String indicated as the content of the error + * @param {ErrorMessage} errorMessage - the error message instance to marshal + * error information into. + * @returns {Undefined} - does not return anything + */ +function populateFromString(ob, errorMessage) { + var fauxError = new Error(); + var fullStack = fauxError.stack.split('\n'); + var cleanedStack = fullStack.slice(0, 1).concat(fullStack.slice(4)); + var errChecked = ''; + + // Replace the generic error message with the user-provided string + cleanedStack[0] = ob; + errChecked = cleanedStack.join('\n'); + + errorMessage.setMessage(errChecked); +} + +/** + * Handles routing and validation for parsing an error which has been indicated + * to be of type Number. This handler will manufacture a new Error to create + * a stack-trace for submission to the Error API and will attempt to caste the + * given number to a string for submission to the Error API. + * @function handleNumberAsError + * @param {Number} err - the number submitted as content for the error message + * @param {ErrorMessage} errorMessage - the error messag instance to marshall + * error information into. + * @returns {Undefined} - does not return anything + */ +function populateFromNumber(ob, errorMessage) { + var fauxError = new Error(); + var errChecked = fauxError.stack; + + if (isNumber(ob) && isFunction(ob.toString)) { + errChecked = ob.toString(); + } + + errorMessage.setMessage(errChecked); +} + +/** + * Handles unknown/unsupported input as the content of the error message. Since + * the problem-space is not defined for this path the library only attempts to + * manufacture a stack trace for submission to the API and discards the input + * that was given as the error content. + * @function handleUnknownAsError + * @param {Any} err - the unknown/unsupported input indicated as the content of + * the error. + * @param {ErrorMessage} errorMessage - the error message instance to marshal + * error information into. + * @returns {Undefined} - does not return anything + */ +function populateFromUnknown(ob, errorMessage) { + var fauxError = new Error(); + + errorMessage.setMessage(fauxError.stack); +} + module.exports = errorHandlerRouter; diff --git a/packages/error-reporting/test/unit/classes/custom-stack-trace.js b/packages/error-reporting/test/unit/classes/custom-stack-trace.js deleted file mode 100644 index 6095ab5051a..00000000000 --- a/packages/error-reporting/test/unit/classes/custom-stack-trace.js +++ /dev/null @@ -1,77 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the 'License'); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an 'AS IS' BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -'use strict'; - -var assert = require('assert'); -var CustomStackTrace = require('../../../src/classes/custom-stack-trace.js'); - -describe('Fuzzing the CustomStackTrace class', function() { - var testFunction = function testFunction() { - return ''; - }; - var cs; - beforeEach(function() { cs = new CustomStackTrace(); }); - it('Should accept value for file path', function() { - cs.setFilePath('test'); - assert( - cs.filePath === 'test', - 'Setting a valid string on the CustomStackTrace.filePath instance ' + - 'should result in assignment' - ); - }); - it('Should reject invalid type for file path', function() { - cs.setFilePath(null); - assert( - cs.filePath === '', - 'Setting an invalid type on the CustomStackTrace.filePath instance ' + - 'should result in default value of an empty string' - ); - }); - it('Should accept value for line number', function() { - cs.setLineNumber(10); - assert( - cs.lineNumber === 10, - 'Setting a valid number on the CustomStackTrace.lineNumber instance ' + - 'should result in assignment' - ); - }); - it('Should reject invalid type for line number', function() { - cs.setLineNumber('10'); - assert( - cs.lineNumber === 0, - 'Setting an invalid type on the CustomStackTrace.lineNumber instance ' + - 'should result in default value of number 0' - ); - }); - it('Should accept value for call list', function() { - cs.setStringifyStructuredCallList(testFunction); - assert.strictEqual( - cs.stringifyStucturedCallList, - testFunction, - 'Setting a valid function on the CustomStackTrace. ' + - 'setStringifyStructuredCallList should result in assignment' - ); - }); - it('Should reject incalid value for call list', function() { - cs.setStringifyStructuredCallList(null); - assert( - ((typeof cs.setStringifyStructuredCallList) === 'function'), - 'Setting an invalid setStringifyStructuredCallList on the ' + - 'CustomStackTrace. setStringifyStructuredCallList should result in a ' + - 'default value of a function' - ); - }); -}); diff --git a/packages/error-reporting/test/unit/error-extractors/error.js b/packages/error-reporting/test/unit/error-extractors/error.js deleted file mode 100644 index cda098e60e7..00000000000 --- a/packages/error-reporting/test/unit/error-extractors/error.js +++ /dev/null @@ -1,107 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the 'License'); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an 'AS IS' BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var extractFromErrorClass = require('../../../src/error-extractors/error.js'); -var ErrorMessage = require('../../../src/classes/error-message.js'); - - -describe('Writing and reading ErrorMessage properties', function() { - describe('Message field', function() { - it('Should set the message as the stack of the given error', function() { - var TEST_MESSAGE = 'This is a test'; - var em = new ErrorMessage(); - var err = new Error(TEST_MESSAGE); - extractFromErrorClass(err, em); - assert.deepEqual(em.message, err.stack, 'Given a valid message the ' + - 'error message should absorb the error stack as the message' - ); - }); - }); - describe('User field', function() { - var em, err; - var TEST_USER_INVALID = 12; - beforeEach(function() { - em = new ErrorMessage(); - err = new Error(); - }); - it('Should set the user field if given valid input', function() { - var TEST_USER_VALID = 'TEST_USER'; - err.user = TEST_USER_VALID; - extractFromErrorClass(err, em); - assert.strictEqual(em.context.user, TEST_USER_VALID); - }); - it('Should default the user field if given invalid input', function() { - err.user = TEST_USER_INVALID; - extractFromErrorClass(err, em); - assert.strictEqual(em.context.user, ''); - }); - }); - describe('Service field', function() { - var em, err; - var TEST_SERVICE_DEFAULT = {service: 'node', version: undefined}; - beforeEach(function() { - em = new ErrorMessage(); - err = new Error(); - }); - it('Should set the field if given valid input', function() { - var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; - err.serviceContext = TEST_SERVICE_VALID; - extractFromErrorClass(err, em); - assert.deepEqual(err.serviceContext, TEST_SERVICE_VALID); - }); - it('Should default the field if given invalid input', function() { - var TEST_SERVICE_INVALID = 12; - err.serviceContext = TEST_SERVICE_INVALID; - extractFromErrorClass(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); - }); - it('Should default the field if not given input', function() { - extractFromErrorClass(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); - }); - }); - describe('Report location field', function() { - var em, err; - var TEST_STACK_DEFAULT = { - filePath: '', - lineNumber: 0, - functionName: '' - }; - beforeEach(function() { - em = new ErrorMessage(); - err = new Error(); - }); - it('Should default the field if given invalid input', function() { - var TEST_STACK_INVALID_CONTENTS = { - filePath: null, - lineNumber: '2', - functionName: {} - }; - err.stack = TEST_STACK_INVALID_CONTENTS; - extractFromErrorClass(err, em); - assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); - }); - it('Should default field if not given a valid type', function() { - var TEST_STACK_INVALID_TYPE = []; - err.stack = TEST_STACK_INVALID_TYPE; - extractFromErrorClass(err, em); - assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); - }); - }); -}); diff --git a/packages/error-reporting/test/unit/error-extractors/object.js b/packages/error-reporting/test/unit/error-extractors/object.js deleted file mode 100644 index bb3d32136ba..00000000000 --- a/packages/error-reporting/test/unit/error-extractors/object.js +++ /dev/null @@ -1,108 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var extractFromObject = require('../../../src/error-extractors/object.js'); -var ErrorMessage = require('../../../src/classes/error-message.js'); - -describe('Object value extraction as error message', function() { - var em, err; - beforeEach(function() { - em = new ErrorMessage(); - err = {}; - }); - describe('Message field', function() { - it('Should write to the field given valid input', function() { - var MESSAGE = 'test'; - err = {message: MESSAGE}; - extractFromObject(err, em); - assert.strictEqual(em.message, MESSAGE); - }); - it('Should default the field given lack-of input', function() { - extractFromObject(err, em); - assert.strictEqual(em.message, ''); - }); - }); - describe('User field', function() { - it('Should write to the field given valid input', function() { - var USER = 'test'; - err.user = USER; - extractFromObject(err, em); - assert.strictEqual(em.context.user, USER); - }); - it('Should default the field given lack-of input', function() { - extractFromObject(err, em); - assert.strictEqual(em.context.user, ''); - }); - }); - describe('filePath field', function() { - it('Should write to the field given valid input', function() { - var PATH = 'test'; - err.filePath = PATH; - extractFromObject(err, em); - assert.strictEqual(em.context.reportLocation.filePath, PATH); - }); - it('Should default the field given lack-of input', function() { - extractFromObject(err, em); - assert.strictEqual(em.context.reportLocation.filePath, ''); - }); - }); - describe('lineNumber field', function() { - it('Should write to the field given valid input', function() { - var LINE_NUMBER = 10; - err.lineNumber = LINE_NUMBER; - extractFromObject(err, em); - assert.strictEqual(em.context.reportLocation.lineNumber, LINE_NUMBER); - }); - it('Should default the field given lack-of input', function() { - extractFromObject(err, em); - assert.strictEqual(em.context.reportLocation.lineNumber, 0); - }); - }); - describe('functionName field', function() { - it('Should write to the field given valid input', function() { - var FUNCTION_NAME = 'test'; - err.functionName = FUNCTION_NAME; - extractFromObject(err, em); - assert.strictEqual(em.context.reportLocation.functionName, FUNCTION_NAME); - }); - it('Should default the field given lack-of input', function() { - extractFromObject(err, em); - assert.strictEqual(em.context.reportLocation.functionName, ''); - }); - }); - describe('serviceContext field', function() { - var TEST_SERVICE_DEFAULT = {service: 'node', version: undefined}; - it('Should write to the field given valid input', function() { - var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; - err.serviceContext = TEST_SERVICE_VALID; - extractFromObject(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_VALID); - }); - it('Should default the field given invalid input', function() { - var TEST_SERVICE_INVALID = 12; - err.serviceContext = TEST_SERVICE_INVALID; - extractFromObject(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); - }); - it('Should default the field given lack-of input', function() { - extractFromObject(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); - }); - }); -}); diff --git a/packages/error-reporting/test/unit/error-handlers/error.js b/packages/error-reporting/test/unit/error-handlers/error.js deleted file mode 100644 index 0e0156d2f07..00000000000 --- a/packages/error-reporting/test/unit/error-handlers/error.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var ErrorMessage = require('../../../src/classes/error-message.js'); -var handleErrorClassError = require('../../../src/error-handlers/error.js'); - -describe('Behaviour under various type inputs', function() { - var em; - var adversarialObjectInput = { - stack: {} - }; - var adversarialObjectInputTwo = { - stack: [] - }; - beforeEach(function() {em = new ErrorMessage();}); - it('Should not throw given undefined', function() { - assert.doesNotThrow(handleErrorClassError.bind(null, undefined, em)); - }); - it('Should not throw given null', function() { - assert.doesNotThrow(handleErrorClassError.bind(null, null, em)); - }); - it('Should not throw given a string', function() { - assert.doesNotThrow(handleErrorClassError.bind(null, 'string_test', em)); - }); - it('Should not throw given a number', function() { - assert.doesNotThrow(handleErrorClassError.bind(null, 1.2, em)); - }); - it('Should not throw given an array', function() { - assert.doesNotThrow(handleErrorClassError.bind(null, [], em)); - }); - it('Should not throw given an object of invalid form', function() { - assert.doesNotThrow( - handleErrorClassError.bind(null, adversarialObjectInput, em)); - assert.doesNotThrow( - handleErrorClassError.bind(null, adversarialObjectInputTwo, em)); - }); - it('Should not throw given valid input', function() { - assert.doesNotThrow(handleErrorClassError.bind(null, new Error(), em)); - }); -}); diff --git a/packages/error-reporting/test/unit/error-handlers/number.js b/packages/error-reporting/test/unit/error-handlers/number.js deleted file mode 100644 index fbdf870edf9..00000000000 --- a/packages/error-reporting/test/unit/error-handlers/number.js +++ /dev/null @@ -1,44 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var ErrorMessage = require('../../../src/classes/error-message.js'); -var handleNumberAsError = require('../../../src/error-handlers/number.js'); - -describe('handleNumberAsError behaviour under varying input', function() { - var em; - beforeEach(function() {em = new ErrorMessage();}); - it('Should not throw given undefined', function() { - assert.doesNotThrow(handleNumberAsError.bind(null, undefined, em)); - }); - it('Should not throw given null', function() { - assert.doesNotThrow(handleNumberAsError.bind(null, null, em)); - }); - it('Should not throw given a string', function() { - assert.doesNotThrow(handleNumberAsError.bind(null, 'test', em)); - }); - it('Should not throw given an instance of Error', function() { - assert.doesNotThrow(handleNumberAsError.bind(null, new Error(), em)); - }); - it('Should not throw given an object', function() { - assert.doesNotThrow(handleNumberAsError.bind(null, {}, em)); - }); - it('Should not throw given valid input', function() { - assert.doesNotThrow(handleNumberAsError.bind(null, 1.3, em)); - }); -}); diff --git a/packages/error-reporting/test/unit/error-handlers/object.js b/packages/error-reporting/test/unit/error-handlers/object.js deleted file mode 100644 index 3e00791e98c..00000000000 --- a/packages/error-reporting/test/unit/error-handlers/object.js +++ /dev/null @@ -1,44 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var ErrorMessage = require('../../../src/classes/error-message.js'); -var handleObjectAsError = require('../../../src/error-handlers/object.js'); - -describe('handleObjectAsError behaviour under varying inputs', function() { - var em; - beforeEach(function() {em = new ErrorMessage();}); - it('Should not throw given undefined', function() { - assert.doesNotThrow(handleObjectAsError.bind(null, undefined, em)); - }); - it('Should not throw given null', function() { - assert.doesNotThrow(handleObjectAsError.bind(null, null, em)); - }); - it('Should not throw given a string', function() { - assert.doesNotThrow(handleObjectAsError.bind(null, 'msg', em)); - }); - it('Should not throw given an instance of Error', function() { - assert.doesNotThrow(handleObjectAsError.bind(null, new Error(), em)); - }); - it('Should not throw given a number', function() { - assert.doesNotThrow(handleObjectAsError.bind(null, 1.3, em)); - }); - it('Should not throw given valid input', function() { - assert.doesNotThrow(handleObjectAsError.bind(null, {}, em)); - }); -}); diff --git a/packages/error-reporting/test/unit/error-handlers/string.js b/packages/error-reporting/test/unit/error-handlers/string.js deleted file mode 100644 index e5ae4704f23..00000000000 --- a/packages/error-reporting/test/unit/error-handlers/string.js +++ /dev/null @@ -1,44 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var ErrorMessage = require('../../../src/classes/error-message.js'); -var handleStringAsError = require('../../../src/error-handlers/string.js'); - -describe('handleStringAsError behaviour under varying inputs', function() { - var em; - beforeEach(function() {em = new ErrorMessage();}); - it('Should not throw given undefined', function() { - assert.doesNotThrow(handleStringAsError.bind(null, undefined, em)); - }); - it('Should not throw given null', function() { - assert.doesNotThrow(handleStringAsError.bind(null, null, em)); - }); - it('Should not throw given an object', function() { - assert.doesNotThrow(handleStringAsError.bind(null, {}, em)); - }); - it('Should not throw given an array', function() { - assert.doesNotThrow(handleStringAsError.bind(null, [], em)); - }); - it('Should not throw given an instance of Error', function() { - assert.doesNotThrow(handleStringAsError.bind(null, 1.3, em)); - }); - it('Should not throw given valid input', function() { - assert.doesNotThrow(handleStringAsError.bind(null, 'test', em)); - }); -}); diff --git a/packages/error-reporting/test/unit/error-handlers/unknown.js b/packages/error-reporting/test/unit/error-handlers/unknown.js deleted file mode 100644 index f971a17b94d..00000000000 --- a/packages/error-reporting/test/unit/error-handlers/unknown.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Copyright 2016 Google Inc. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -'use strict'; - -var assert = require('assert'); -var ErrorMessage = require('../../../src/classes/error-message.js'); -var handleUnknownAsError = require('../../../src/error-handlers/unknown.js'); - -describe('handleUnknownAsError behvaiour under varying input', function() { - var em; - beforeEach(function() {em = new ErrorMessage();}); - it('Should not throw given undefined', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, undefined, em)); - }); - it('Should not throw given null', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, null, em)); - }); - it('Should not throw given an object', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, {}, em)); - }); - it('Should not throw given an array', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, [], em)); - }); - it('Should not throw given an instance of Error', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, new Error(), em)); - }); - it('Should not throw given a number', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, 1.3, em)); - }); - it('Should not throw given a string', function() { - assert.doesNotThrow(handleUnknownAsError.bind(null, 'msg', em)); - }); -}); diff --git a/packages/error-reporting/test/unit/error-router.js b/packages/error-reporting/test/unit/error-router.js new file mode 100644 index 00000000000..22c66fdecad --- /dev/null +++ b/packages/error-reporting/test/unit/error-router.js @@ -0,0 +1,236 @@ +/** + * Copyright 2017 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +var assert = require('assert'); + +var ErrorMessage = require('../../src/classes/error-message.js'); +var errorHandlerRouter = require('../../src/error-router.js'); + +var TEST_USER_INVALID = 12; +var TEST_MESSAGE = 'This is a test'; +var TEST_SERVICE_DEFAULT = {service: 'node', version: undefined}; +var TEST_STACK_DEFAULT = { + filePath: '', + lineNumber: 0, + functionName: '' + }; + +describe('error-router', function() { + var em; + var adversarialObjectInput = { + stack: {} + }; + var adversarialObjectInputTwo = { + stack: [] + }; + beforeEach(function() { + em = new ErrorMessage(); + }); + + it('Should not throw given undefined', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, undefined, em)); + }); + + it('Should not throw given null', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, null, em)); + }); + + it('Should not throw given a string', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, 'string_test', em)); + }); + + it('Should not throw given a number', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, 1.2, em)); + }); + + it('Should not throw given an array', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, [], em)); + }); + + it('Should not throw given an object', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, {}, em)); + }); + + it('Should not throw given an instance of Error', function() { + assert.doesNotThrow(errorHandlerRouter.bind(null, new Error(), em)); + }); + + it('Should not throw given an object of invalid form', function() { + assert.doesNotThrow( + errorHandlerRouter.bind(null, adversarialObjectInput, em)); + assert.doesNotThrow( + errorHandlerRouter.bind(null, adversarialObjectInputTwo, em)); + }); + + it('Message field: Should set the message as the stack of the given error', function() { + var err = new Error(TEST_MESSAGE); + errorHandlerRouter(err, em); + assert.deepEqual(em.message, err.stack, 'Given a valid message the ' + + 'error message should absorb the error stack as the message' + ); + }); + + it('User field: Should set the user field if given valid input', function() { + var err = new Error(); + var TEST_USER_VALID = 'TEST_USER'; + err.user = TEST_USER_VALID; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.user, TEST_USER_VALID); + }); + + it('User field: Should default the user field if given invalid input', function() { + var err = new Error(); + err.user = TEST_USER_INVALID; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.user, ''); + }); + + it('Service field: Should set the field if given valid input', function() { + var err = new Error(); + var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; + err.serviceContext = TEST_SERVICE_VALID; + errorHandlerRouter(err, em); + assert.deepEqual(err.serviceContext, TEST_SERVICE_VALID); + }); + + it('Service field: Should default the field if given invalid input', function() { + var err = new Error(); + var TEST_SERVICE_INVALID = 12; + err.serviceContext = TEST_SERVICE_INVALID; + errorHandlerRouter(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); + }); + + it('Service field: Should default the field if not given input', function() { + var err = new Error(); + errorHandlerRouter(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); + }); + + it('Report location field: Should default the field if given invalid input', function() { + var TEST_STACK_INVALID_CONTENTS = { + filePath: null, + lineNumber: '2', + functionName: {} + }; + var err = new Error(); + err.stack = TEST_STACK_INVALID_CONTENTS; + errorHandlerRouter(err, em); + assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); + }); + + it('Report location field: Should default field if not given a valid type', function() { + var err = new Error(); + var TEST_STACK_INVALID_TYPE = []; + err.stack = TEST_STACK_INVALID_TYPE; + errorHandlerRouter(err, em); + assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); + }); + + it('Message Field: Should write to the field given valid input', function() { + var err = {}; + var MESSAGE = 'test'; + err = {message: MESSAGE}; + errorHandlerRouter(err, em); + assert.strictEqual(em.message, MESSAGE); + }); + + it('Message Field: Should default the field given lack-of input', function() { + var err = {}; + errorHandlerRouter(err, em); + assert.strictEqual(em.message, ''); + }); + + it('User field: Should write to the field given valid input', function() { + var err = {}; + var USER = 'test'; + err.user = USER; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.user, USER); + }); + + it('User field: Should default the field given lack-of input', function() { + var err = {}; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.user, ''); + }); + + it('FilePath Field: Should write to the field given valid input', function() { + var err = {}; + var PATH = 'test'; + err.filePath = PATH; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.reportLocation.filePath, PATH); + }); + + it('FilePath Field: Should default the field given lack-of input', function() { + var err = {}; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.reportLocation.filePath, ''); + }); + + it('LineNumber Field: Should write to the field given valid input', function() { + var err = {}; + var LINE_NUMBER = 10; + err.lineNumber = LINE_NUMBER; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.reportLocation.lineNumber, LINE_NUMBER); + }); + + it('LineNumber Field: Should default the field given lack-of input', function() { + var err = {}; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.reportLocation.lineNumber, 0); + }); + + it('FunctionName Field: Should write to the field given valid input', function() { + var err = {}; + var FUNCTION_NAME = 'test'; + err.functionName = FUNCTION_NAME; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.reportLocation.functionName, FUNCTION_NAME); + }); + + it('FunctionName Field: Should default the field given lack-of input', function() { + var err = {}; + errorHandlerRouter(err, em); + assert.strictEqual(em.context.reportLocation.functionName, ''); + }); + + it('ServiceContext Field: Should write to the field given valid input', function() { + var err = {}; + var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; + err.serviceContext = TEST_SERVICE_VALID; + errorHandlerRouter(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_VALID); + }); + + it('ServiceContext Field: Should default the field given invalid input', function() { + var err = {}; + var TEST_SERVICE_INVALID = 12; + err.serviceContext = TEST_SERVICE_INVALID; + errorHandlerRouter(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); + }); + + it('ServiceContext Field: Should default the field given lack-of input', function() { + var err = {}; + errorHandlerRouter(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); + }); +}); From 61ff46c2e2cc8a90023de59b2ad59e2410d9b7c3 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Tue, 6 Jun 2017 13:55:32 -0700 Subject: [PATCH 02/11] Fix linting errors --- packages/error-reporting/src/error-router.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/error-reporting/src/error-router.js b/packages/error-reporting/src/error-router.js index 78ef83d45f1..e533d9726b0 100644 --- a/packages/error-reporting/src/error-router.js +++ b/packages/error-reporting/src/error-router.js @@ -35,20 +35,15 @@ var isFunction = is.function; function errorHandlerRouter(ob, em) { if (ob === null || ob === undefined) { populateFromUnknown(ob, em); - } - else if (ob instanceof Error) { + } else if (ob instanceof Error) { populateFromError(ob, em); - } - else if (typeof ob === 'object' && isObject(ob)) { + } else if (typeof ob === 'object' && isObject(ob)) { populateFromObject(ob, em); - } - else if (typeof ob === 'string' && isString(ob)) { + } else if (typeof ob === 'string' && isString(ob)) { populateFromString(ob, em); - } - else if (typeof ob === 'number') { + } else if (typeof ob === 'number') { populateFromNumber(ob, em); - } - else { + } else { populateFromUnknown(ob, em); } From 3942bb7f1e95f74113462d61548d643645e8f6f9 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Tue, 6 Jun 2017 15:04:52 -0700 Subject: [PATCH 03/11] Add the new `build-stack-trace.js` file --- .../error-reporting/src/build-stack-trace.js | 25 ++++++++ packages/error-reporting/src/error-router.js | 26 ++------ .../src/interfaces/message-builder.js | 5 +- .../test/unit/build-stack-trace.js | 60 +++++++++++++++++++ 4 files changed, 92 insertions(+), 24 deletions(-) create mode 100644 packages/error-reporting/src/build-stack-trace.js create mode 100644 packages/error-reporting/test/unit/build-stack-trace.js diff --git a/packages/error-reporting/src/build-stack-trace.js b/packages/error-reporting/src/build-stack-trace.js new file mode 100644 index 00000000000..e8a173342fd --- /dev/null +++ b/packages/error-reporting/src/build-stack-trace.js @@ -0,0 +1,25 @@ +/** + * Copyright 2017 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +function buildStackTrace(message) { + var fauxError = new Error(''); + var fullStack = fauxError.stack.split('\n'); + return (message ? message + '\n' : '') + fullStack.slice(2).join('\n'); +} + +module.exports = buildStackTrace; diff --git a/packages/error-reporting/src/error-router.js b/packages/error-reporting/src/error-router.js index e533d9726b0..3aa998fe2aa 100644 --- a/packages/error-reporting/src/error-router.js +++ b/packages/error-reporting/src/error-router.js @@ -21,6 +21,7 @@ var isObject = is.object; var isString = is.string; var isNumber = is.number; var isFunction = is.function; +var buildStackTrace = require('./build-stack-trace.js'); /** * The Error handler router is responsible for taking an object of some type and @@ -137,16 +138,7 @@ function populateFromObject(ob, errorMessage) { * @returns {Undefined} - does not return anything */ function populateFromString(ob, errorMessage) { - var fauxError = new Error(); - var fullStack = fauxError.stack.split('\n'); - var cleanedStack = fullStack.slice(0, 1).concat(fullStack.slice(4)); - var errChecked = ''; - - // Replace the generic error message with the user-provided string - cleanedStack[0] = ob; - errChecked = cleanedStack.join('\n'); - - errorMessage.setMessage(errChecked); + errorMessage.setMessage(buildStackTrace(ob)); } /** @@ -161,14 +153,8 @@ function populateFromString(ob, errorMessage) { * @returns {Undefined} - does not return anything */ function populateFromNumber(ob, errorMessage) { - var fauxError = new Error(); - var errChecked = fauxError.stack; - - if (isNumber(ob) && isFunction(ob.toString)) { - errChecked = ob.toString(); - } - - errorMessage.setMessage(errChecked); + var message = isNumber(ob) && isFunction(ob.toString) ? ob.toString() : ''; + errorMessage.setMessage(buildStackTrace(message)); } /** @@ -184,9 +170,7 @@ function populateFromNumber(ob, errorMessage) { * @returns {Undefined} - does not return anything */ function populateFromUnknown(ob, errorMessage) { - var fauxError = new Error(); - - errorMessage.setMessage(fauxError.stack); + errorMessage.setMessage(buildStackTrace()); } module.exports = errorHandlerRouter; diff --git a/packages/error-reporting/src/interfaces/message-builder.js b/packages/error-reporting/src/interfaces/message-builder.js index 3a539d3b3be..1ea82f1d045 100644 --- a/packages/error-reporting/src/interfaces/message-builder.js +++ b/packages/error-reporting/src/interfaces/message-builder.js @@ -16,6 +16,7 @@ 'use strict'; var ErrorMessage = require('../classes/error-message.js'); +var buildStackTrace = require('../build-stack-trace.js'); /** * The handler setup function serves to produce a bound instance of the @@ -44,9 +45,7 @@ function handlerSetup(config) { // error is used instead of the stack trace where the error is // reported to be consistent with the behavior of reporting a // an error when reporting an actual Node.js Error object. - var fauxError = new Error(''); - var fullStack = fauxError.stack.split('\n'); - var cleanedStack = fullStack.slice(2).join('\n'); + var cleanedStack = buildStackTrace(); var em = new ErrorMessage().setServiceContext( config.getServiceContext().service, diff --git a/packages/error-reporting/test/unit/build-stack-trace.js b/packages/error-reporting/test/unit/build-stack-trace.js new file mode 100644 index 00000000000..6337ebee714 --- /dev/null +++ b/packages/error-reporting/test/unit/build-stack-trace.js @@ -0,0 +1,60 @@ +/** + * Copyright 2017 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +'use strict'; + +var assert = require('assert'); +var buildStackTrace = require('../../src/build-stack-trace.js'); + +describe('build-stack-trace', function() { + it('Should not have a message attached if none is given', function() { + assert(buildStackTrace().startsWith(' at')); + assert(!buildStackTrace(undefined).startsWith('undefined')); + assert(!buildStackTrace(null).startsWith('null')); + }); + + it('Should attach a message if given', function() { + assert(buildStackTrace('Some Message').startsWith('Some Message\n')); + }); + + it('Should not contain error-reporting specific frames', function() { + var internalFileName = 'build-stack-trace'; + var stackTrace = buildStackTrace(); + var firstIndex = stackTrace.indexOf(internalFileName); + var lastIndex = stackTrace.lastIndexOf(internalFileName); + // This file, named 'build-stack-trace.js', tests the + // 'build-stack-trace.js' file. The stack trace should not contain + // information about the 'build-stack-trace.js' file that is being + // tested. Thus the stack trace should only contain the string + // 'build-stack-trace' one time, which corresponds to this test file + // and not the 'build-stack-trace.js' file being tested. + assert.strictEqual(firstIndex, lastIndex); + }); + + it('Should return the stack trace', function() { + (function functionA() { + (function functionB() { + (function functionC() { + var stackTrace = buildStackTrace(); + assert(stackTrace); + assert.notStrictEqual(stackTrace.indexOf('functionA'), -1); + assert.notStrictEqual(stackTrace.indexOf('functionB'), -1); + assert.notStrictEqual(stackTrace.indexOf('functionC'), -1); + })(); + })(); + })(); + }); +}); From d40402c41217c182b6d517cd48af21ba8ff7d6ca Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 7 Jun 2017 00:01:58 -0700 Subject: [PATCH 04/11] Add more system tests for `errors.report()` --- .../error-reporting/src/build-stack-trace.js | 24 ++++++- packages/error-reporting/src/error-router.js | 47 +++++++------- .../src/interfaces/message-builder.js | 2 +- .../system-test/testAuthClient.js | 65 +++++++++++++++---- .../test/unit/build-stack-trace.js | 35 ++++++++-- 5 files changed, 133 insertions(+), 40 deletions(-) diff --git a/packages/error-reporting/src/build-stack-trace.js b/packages/error-reporting/src/build-stack-trace.js index e8a173342fd..cd9571dc403 100644 --- a/packages/error-reporting/src/build-stack-trace.js +++ b/packages/error-reporting/src/build-stack-trace.js @@ -16,10 +16,30 @@ 'use strict'; -function buildStackTrace(message) { +var is = require('is'); + +/** + * Constructs a string representation of the stack trace at the point where + * this function was invoked. Note that the stack trace will not include any + * references to this function itself. + * @function buildStackTrace + * @param {String?} message - The message that should appear as the first line + * of the stack trace. This value defaults to the empty string. + * @param {Number?} numSkip - The number of additional lines to not display + * in the stack trace. That is, the stack trace returned will have the + * specified message with the top `numSkip` lines of the stack not displayed. + * This value defaults to 0 if it is not defined or is not a number. + * @returns {String} - A string representation of the stack trace at the point + * where this method was invoked. + */ +function buildStackTrace(message, numSkip) { var fauxError = new Error(''); var fullStack = fauxError.stack.split('\n'); - return (message ? message + '\n' : '') + fullStack.slice(2).join('\n'); + var prefix = message ? message + '\n' : ''; + // Two additional lines are skipped to skip the default empty message + // and the line referencing this function itself. + var totalNumSkip = (is.number(numSkip) ? numSkip : 0) + 2; + return prefix + fullStack.slice(totalNumSkip).join('\n'); } module.exports = buildStackTrace; diff --git a/packages/error-reporting/src/error-router.js b/packages/error-reporting/src/error-router.js index 3aa998fe2aa..933b8ee52a7 100644 --- a/packages/error-reporting/src/error-router.js +++ b/packages/error-reporting/src/error-router.js @@ -57,6 +57,7 @@ function errorHandlerRouter(ob, em) { * will check before accessing any part of the error for propety presence but * will not check the types of these property values that is instead work that * is allocated to the error message instance itself. + * @function populateFromError * @param {Error} err - the error instance * @param {ErrorMessage} errorMessage - the error message instance to have the * error information marshaled into @@ -80,20 +81,20 @@ function populateFromError(err, errorMessage) { * error. This function will check presence of each property before attempting * to access the given property on the object but will not check for type * compliance as that is allocated to the instance of the error message itself. - * @function extractFromObject - * @param {Object} err - the Object given as the content of the error - * @param {String} [err.message] - the error message - * @param {String} [err.user] - the user the error occurred for - * @param {String} [err.filePath] - the file path and file where the error + * @function populateFromObject + * @param {Object} ob - the Object given as the content of the error + * @param {String} [ob.message] - the error message + * @param {String} [ob.user] - the user the error occurred for + * @param {String} [ob.filePath] - the file path and file where the error * occurred at - * @param {Number} [err.lineNumber] - the line number where the error occurred + * @param {Number} [ob.lineNumber] - the line number where the error occurred * at - * @param {String} [err.functionName] - the function where the error occurred at - * @param {Object} [err.serviceContext] - the service context object of the + * @param {String} [ob.functionName] - the function where the error occurred at + * @param {Object} [ob.serviceContext] - the service context object of the * error - * @param {String} [err.serviceContext.service] - the service the error occurred + * @param {String} [ob.serviceContext.service] - the service the error occurred * on - * @param {String} [err.serviceContext.version] - the version of the application + * @param {String} [ob.serviceContext.version] - the version of the application * that the error occurred on * @param {ErrorMessage} errorMessage - the error message instance to marshal * error information into @@ -102,6 +103,8 @@ function populateFromError(err, errorMessage) { function populateFromObject(ob, errorMessage) { if (has(ob, 'message')) { errorMessage.setMessage(ob.message); + } else { + errorMessage.setMessage(buildStackTrace('' + ob, 3)); } if (has(ob, 'user')) { @@ -131,14 +134,14 @@ function populateFromObject(ob, errorMessage) { * This function will create a new instance of the Error class to produce a * stack trace for submission to the API and check to confirm that the given * value is of type string. - * @function handleStringAsError - * @param {String} err - the String indicated as the content of the error + * @function populateFromString + * @param {String} str - the String indicated as the content of the error * @param {ErrorMessage} errorMessage - the error message instance to marshal * error information into. * @returns {Undefined} - does not return anything */ -function populateFromString(ob, errorMessage) { - errorMessage.setMessage(buildStackTrace(ob)); +function populateFromString(str, errorMessage) { + errorMessage.setMessage(buildStackTrace(str, 3)); } /** @@ -146,15 +149,15 @@ function populateFromString(ob, errorMessage) { * to be of type Number. This handler will manufacture a new Error to create * a stack-trace for submission to the Error API and will attempt to caste the * given number to a string for submission to the Error API. - * @function handleNumberAsError - * @param {Number} err - the number submitted as content for the error message + * @function populateFromNumber + * @param {Number} num - the number submitted as content for the error message * @param {ErrorMessage} errorMessage - the error messag instance to marshall * error information into. * @returns {Undefined} - does not return anything */ -function populateFromNumber(ob, errorMessage) { - var message = isNumber(ob) && isFunction(ob.toString) ? ob.toString() : ''; - errorMessage.setMessage(buildStackTrace(message)); +function populateFromNumber(num, errorMessage) { + var message = isNumber(num) && isFunction(num.toString) ? num.toString() : ''; + errorMessage.setMessage(buildStackTrace(message, 3)); } /** @@ -162,15 +165,15 @@ function populateFromNumber(ob, errorMessage) { * the problem-space is not defined for this path the library only attempts to * manufacture a stack trace for submission to the API and discards the input * that was given as the error content. - * @function handleUnknownAsError - * @param {Any} err - the unknown/unsupported input indicated as the content of + * @function populateFromUnknown + * @param {Any} ob - the unknown/unsupported input indicated as the content of * the error. * @param {ErrorMessage} errorMessage - the error message instance to marshal * error information into. * @returns {Undefined} - does not return anything */ function populateFromUnknown(ob, errorMessage) { - errorMessage.setMessage(buildStackTrace()); + errorMessage.setMessage(buildStackTrace('' + ob, 3)); } module.exports = errorHandlerRouter; diff --git a/packages/error-reporting/src/interfaces/message-builder.js b/packages/error-reporting/src/interfaces/message-builder.js index 1ea82f1d045..6cd4e470eb8 100644 --- a/packages/error-reporting/src/interfaces/message-builder.js +++ b/packages/error-reporting/src/interfaces/message-builder.js @@ -45,7 +45,7 @@ function handlerSetup(config) { // error is used instead of the stack trace where the error is // reported to be consistent with the behavior of reporting a // an error when reporting an actual Node.js Error object. - var cleanedStack = buildStackTrace(); + var cleanedStack = buildStackTrace('', 1); var em = new ErrorMessage().setServiceContext( config.getServiceContext().service, diff --git a/packages/error-reporting/system-test/testAuthClient.js b/packages/error-reporting/system-test/testAuthClient.js index a97e2198842..087b1b7180c 100644 --- a/packages/error-reporting/system-test/testAuthClient.js +++ b/packages/error-reporting/system-test/testAuthClient.js @@ -367,8 +367,8 @@ describe('error-reporting', function() { return [Date.now(), BASE_NAME, suffix].join('_'); } - const SERVICE_NAME = buildName('service-name'); - const SERVICE_VERSION = buildName('service-version'); + const SERVICE = buildName('service-name'); + const VERSION = buildName('service-version'); var errors; var transport; @@ -396,8 +396,8 @@ describe('error-reporting', function() { var config = Object.assign({ ignoreEnvironmentCheck: true, serviceContext: { - service: SERVICE_NAME, - version: SERVICE_VERSION + service: SERVICE, + version: VERSION } }, extraConfig || {}); errors = require('../src/index.js')(config); @@ -426,6 +426,9 @@ describe('error-reporting', function() { var matchedErrors = groups.filter(function(errItem) { return errItem && errItem.representative && + errItem.representative.serviceContext && + errItem.representative.serviceContext.service === SERVICE && + errItem.representative.serviceContext.version === VERSION && messageTest(errItem.representative.message); }); @@ -445,8 +448,8 @@ describe('error-reporting', function() { assert.ok(rep); var context = rep.serviceContext; assert.ok(context); - assert.strictEqual(context.service, SERVICE_NAME); - assert.strictEqual(context.version, SERVICE_VERSION); + assert.strictEqual(context.service, SERVICE); + assert.strictEqual(context.version, VERSION); cb(); }); } @@ -466,21 +469,61 @@ describe('error-reporting', function() { // As such, each test is set to fail due to a timeout only if sufficiently // more than TIMEOUT ms has elapsed to avoid test fragility. - it('Should correctly publish errors using the Error constructor', - function(done) { + it('Should correctly publish an error that is an Error object', + function verifyErrors(done) { this.timeout(TIMEOUT * 2); var errorId = buildName('with-error-constructor'); var errOb = new Error(errorId); verifyReporting(errOb, function(message) { - return message.startsWith('Error: ' + errorId); + return message.startsWith('Error: ' + errorId + + '\n at Context.verifyErrors'); }, TIMEOUT, done); }); - it('Should correctly publish errors using a string', function(done) { + it('Should correctly publish an error that is a string', function(done) { this.timeout(TIMEOUT * 2); var errorId = buildName('with-string'); verifyReporting(errorId, function(message) { - return message.startsWith(errorId); + return message.startsWith(errorId + '\n at verifyReporting'); + }, TIMEOUT, done); + }); + + it('Should correctly publish an error that is undefined', function(done) { + this.timeout(TIMEOUT * 2); + verifyReporting(undefined, function(message) { + return message.startsWith('undefined\n at verifyReporting'); + }, TIMEOUT, done); + }); + + it('Should correctly publish an error that is null', function(done) { + this.timeout(TIMEOUT * 2); + verifyReporting(null, function(message) { + return message.startsWith('null\n at verifyReporting'); + }, TIMEOUT, done); + }); + + it('Should correctly publish an error that is a plain object', + function(done) { + this.timeout(TIMEOUT * 2); + verifyReporting({ someKey: 'someValue' }, function(message) { + return message.startsWith('[object Object]\n at verifyReporting'); + }, TIMEOUT, done); + }); + + it('Should correctly publish an error that is a number', function(done) { + this.timeout(TIMEOUT * 2); + var num = (new Date()).getTime(); + verifyReporting(num, function(message) { + return message.startsWith('' + num + '\n at verifyReporting'); + }, TIMEOUT, done); + }); + + it('Should correctly publish an error that is of an unknown type', + function(done) { + this.timeout(TIMEOUT * 2); + var bool = true; + verifyReporting(bool, function(message) { + return message.startsWith('true\n at verifyReporting'); }, TIMEOUT, done); }); diff --git a/packages/error-reporting/test/unit/build-stack-trace.js b/packages/error-reporting/test/unit/build-stack-trace.js index 6337ebee714..c3706ad56d4 100644 --- a/packages/error-reporting/test/unit/build-stack-trace.js +++ b/packages/error-reporting/test/unit/build-stack-trace.js @@ -20,17 +20,27 @@ var assert = require('assert'); var buildStackTrace = require('../../src/build-stack-trace.js'); describe('build-stack-trace', function() { - it('Should not have a message attached if none is given', function() { + it('Should not have a message attached if none is given without specifying to skip frames', function() { assert(buildStackTrace().startsWith(' at')); assert(!buildStackTrace(undefined).startsWith('undefined')); assert(!buildStackTrace(null).startsWith('null')); }); - it('Should attach a message if given', function() { + it('Should not have a message attached if none is given with specifying to skip frames', function() { + assert(buildStackTrace().startsWith(' at'), 2); + assert(!buildStackTrace(undefined).startsWith('undefined'), 2); + assert(!buildStackTrace(null).startsWith('null'), 2); + }); + + it('Should attach a message if given without specifying to skip frames', function() { assert(buildStackTrace('Some Message').startsWith('Some Message\n')); }); - it('Should not contain error-reporting specific frames', function() { + it('Should attach a message if given with specifying to skip frames', function() { + assert(buildStackTrace('Some Message').startsWith('Some Message\n'), 2); + }); + + it('Should not contain error-reporting specific frames without specifying to skip frames', function() { var internalFileName = 'build-stack-trace'; var stackTrace = buildStackTrace(); var firstIndex = stackTrace.indexOf(internalFileName); @@ -44,7 +54,7 @@ describe('build-stack-trace', function() { assert.strictEqual(firstIndex, lastIndex); }); - it('Should return the stack trace', function() { + it('Should return the stack trace without specifying to skip frames', function() { (function functionA() { (function functionB() { (function functionC() { @@ -57,4 +67,21 @@ describe('build-stack-trace', function() { })(); })(); }); + + it('Should return the stack trace with the correct number of frames skipped', function() { + (function functionA() { + (function functionB() { + (function functionC() { + var stackTrace = buildStackTrace('', 2); + assert(stackTrace); + // The stack trace should not contain the frames for + // functionC or functionB because 2 frames were specified as being + // skipped. It should, however, contain functionA. + assert.notStrictEqual(stackTrace.indexOf('functionA'), -1); + assert.strictEqual(stackTrace.indexOf('functionB'), -1); + assert.strictEqual(stackTrace.indexOf('functionC'), -1); + })(); + })(); + })(); + }); }); From f1f58fd561b2a94f0f499cb36a2e06a15085cd6c Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 7 Jun 2017 00:20:50 -0700 Subject: [PATCH 05/11] Rename errorHandlerRouter to populateErrorMessage --- packages/error-reporting/src/error-router.js | 6 +- .../error-reporting/src/interfaces/express.js | 4 +- .../error-reporting/src/interfaces/hapi.js | 4 +- .../error-reporting/src/interfaces/koa.js | 4 +- .../error-reporting/src/interfaces/manual.js | 4 +- .../error-reporting/src/interfaces/restify.js | 4 +- .../error-reporting/test/unit/error-router.js | 64 +++++++++---------- 7 files changed, 45 insertions(+), 45 deletions(-) diff --git a/packages/error-reporting/src/error-router.js b/packages/error-reporting/src/error-router.js index 933b8ee52a7..bf3ad343803 100644 --- a/packages/error-reporting/src/error-router.js +++ b/packages/error-reporting/src/error-router.js @@ -27,13 +27,13 @@ var buildStackTrace = require('./build-stack-trace.js'); * The Error handler router is responsible for taking an object of some type and * and Error message container, analyzing the type of the object and marshalling * the object's information into the error message container. - * @function errorHandlerRouter + * @function populateErrorMessage * @param {Any} ob - the object information to extract from * @param {ErrorMessage} em - an instance of ErrorMessage to marshal object * information into * @returns {Undefined} - does not return a value */ -function errorHandlerRouter(ob, em) { +function populateErrorMessage(ob, em) { if (ob === null || ob === undefined) { populateFromUnknown(ob, em); } else if (ob instanceof Error) { @@ -176,4 +176,4 @@ function populateFromUnknown(ob, errorMessage) { errorMessage.setMessage(buildStackTrace('' + ob, 3)); } -module.exports = errorHandlerRouter; +module.exports = populateErrorMessage; diff --git a/packages/error-reporting/src/interfaces/express.js b/packages/error-reporting/src/interfaces/express.js index 07803c173fe..4779be205e1 100644 --- a/packages/error-reporting/src/interfaces/express.js +++ b/packages/error-reporting/src/interfaces/express.js @@ -21,7 +21,7 @@ var isFunction = is.fn; var ErrorMessage = require('../classes/error-message.js'); var expressRequestInformationExtractor = require('../request-extractors/express.js'); -var errorHandlerRouter = require('../error-router.js'); +var populateErrorMessage = require('../error-router.js'); /** * Returns a function that can be used as an express error handling middleware. @@ -58,7 +58,7 @@ function makeExpressHandler(client, config) { expressRequestInformationExtractor(req, res)) .setServiceContext(ctxService, ctxVersion); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); if (isObject(client) && isFunction(client.sendError)) { client.sendError(em); diff --git a/packages/error-reporting/src/interfaces/hapi.js b/packages/error-reporting/src/interfaces/hapi.js index dcc5999e1b9..18d8c4291c6 100644 --- a/packages/error-reporting/src/interfaces/hapi.js +++ b/packages/error-reporting/src/interfaces/hapi.js @@ -20,7 +20,7 @@ var isObject = is.object; var isFunction = is.fn; var ErrorMessage = require('../classes/error-message.js'); var hapiRequestInformationExtractor = require('../request-extractors/hapi.js'); -var errorHandlerRouter = require('../error-router.js'); +var populateErrorMessage = require('../error-router.js'); var packageJson = require('../../package.json'); /** @@ -46,7 +46,7 @@ function hapiErrorHandler(req, err, config) { .consumeRequestInformation(hapiRequestInformationExtractor(req)) .setServiceContext(service, version); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); return em; } diff --git a/packages/error-reporting/src/interfaces/koa.js b/packages/error-reporting/src/interfaces/koa.js index b0297fb7eaf..450da5d267a 100644 --- a/packages/error-reporting/src/interfaces/koa.js +++ b/packages/error-reporting/src/interfaces/koa.js @@ -17,7 +17,7 @@ 'use strict'; var ErrorMessage = require('../classes/error-message.js'); var koaRequestInformationExtractor = require('../request-extractors/koa.js'); -var errorHandlerRouter = require('../error-router.js'); +var populateErrorMessage = require('../error-router.js'); /** * The koaErrorHandler should be placed at the beginning of the koa middleware @@ -53,7 +53,7 @@ function koaErrorHandler(client, config) { .setServiceContext(svc.service, svc.version); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); client.sendError(em); } diff --git a/packages/error-reporting/src/interfaces/manual.js b/packages/error-reporting/src/interfaces/manual.js index 5be883c979f..6d686ce849d 100644 --- a/packages/error-reporting/src/interfaces/manual.js +++ b/packages/error-reporting/src/interfaces/manual.js @@ -22,7 +22,7 @@ var isFunction = is.fn; var ErrorMessage = require('../classes/error-message.js'); var manualRequestInformationExtractor = require('../request-extractors/manual.js'); -var errorHandlerRouter = require('../error-router.js'); +var populateErrorMessage = require('../error-router.js'); /** * The handler setup function serves to produce a bound instance of the @@ -96,7 +96,7 @@ function handlerSetup(client, config, logger) { em = new ErrorMessage(); em.setServiceContext(config.getServiceContext().service, config.getServiceContext().version); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); } if (isObject(request)) { diff --git a/packages/error-reporting/src/interfaces/restify.js b/packages/error-reporting/src/interfaces/restify.js index 5d8a6e89d6f..3ff4ac072aa 100644 --- a/packages/error-reporting/src/interfaces/restify.js +++ b/packages/error-reporting/src/interfaces/restify.js @@ -21,7 +21,7 @@ var isFunction = is.function; var ErrorMessage = require('../classes/error-message.js'); var expressRequestInformationExtractor = require('../request-extractors/express.js'); -var errorHandlerRouter = require('../error-router.js'); +var populateErrorMessage = require('../error-router.js'); /** * The restifyErrorHandler is responsible for taking the captured error, setting @@ -41,7 +41,7 @@ function restifyErrorHandler(client, config, err, em) { var svc = config.getServiceContext(); em.setServiceContext(svc.service, svc.version); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); client.sendError(em); } diff --git a/packages/error-reporting/test/unit/error-router.js b/packages/error-reporting/test/unit/error-router.js index 22c66fdecad..dac601db8be 100644 --- a/packages/error-reporting/test/unit/error-router.js +++ b/packages/error-reporting/test/unit/error-router.js @@ -19,7 +19,7 @@ var assert = require('assert'); var ErrorMessage = require('../../src/classes/error-message.js'); -var errorHandlerRouter = require('../../src/error-router.js'); +var populateErrorMessage = require('../../src/error-router.js'); var TEST_USER_INVALID = 12; var TEST_MESSAGE = 'This is a test'; @@ -43,43 +43,43 @@ describe('error-router', function() { }); it('Should not throw given undefined', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, undefined, em)); + assert.doesNotThrow(populateErrorMessage.bind(null, undefined, em)); }); it('Should not throw given null', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, null, em)); + assert.doesNotThrow(populateErrorMessage.bind(null, null, em)); }); it('Should not throw given a string', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, 'string_test', em)); + assert.doesNotThrow(populateErrorMessage.bind(null, 'string_test', em)); }); it('Should not throw given a number', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, 1.2, em)); + assert.doesNotThrow(populateErrorMessage.bind(null, 1.2, em)); }); it('Should not throw given an array', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, [], em)); + assert.doesNotThrow(populateErrorMessage.bind(null, [], em)); }); it('Should not throw given an object', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, {}, em)); + assert.doesNotThrow(populateErrorMessage.bind(null, {}, em)); }); it('Should not throw given an instance of Error', function() { - assert.doesNotThrow(errorHandlerRouter.bind(null, new Error(), em)); + assert.doesNotThrow(populateErrorMessage.bind(null, new Error(), em)); }); it('Should not throw given an object of invalid form', function() { assert.doesNotThrow( - errorHandlerRouter.bind(null, adversarialObjectInput, em)); + populateErrorMessage.bind(null, adversarialObjectInput, em)); assert.doesNotThrow( - errorHandlerRouter.bind(null, adversarialObjectInputTwo, em)); + populateErrorMessage.bind(null, adversarialObjectInputTwo, em)); }); it('Message field: Should set the message as the stack of the given error', function() { var err = new Error(TEST_MESSAGE); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.message, err.stack, 'Given a valid message the ' + 'error message should absorb the error stack as the message' ); @@ -89,14 +89,14 @@ describe('error-router', function() { var err = new Error(); var TEST_USER_VALID = 'TEST_USER'; err.user = TEST_USER_VALID; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.user, TEST_USER_VALID); }); it('User field: Should default the user field if given invalid input', function() { var err = new Error(); err.user = TEST_USER_INVALID; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.user, ''); }); @@ -104,7 +104,7 @@ describe('error-router', function() { var err = new Error(); var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; err.serviceContext = TEST_SERVICE_VALID; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(err.serviceContext, TEST_SERVICE_VALID); }); @@ -112,13 +112,13 @@ describe('error-router', function() { var err = new Error(); var TEST_SERVICE_INVALID = 12; err.serviceContext = TEST_SERVICE_INVALID; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); }); it('Service field: Should default the field if not given input', function() { var err = new Error(); - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); }); @@ -130,7 +130,7 @@ describe('error-router', function() { }; var err = new Error(); err.stack = TEST_STACK_INVALID_CONTENTS; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); }); @@ -138,7 +138,7 @@ describe('error-router', function() { var err = new Error(); var TEST_STACK_INVALID_TYPE = []; err.stack = TEST_STACK_INVALID_TYPE; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); }); @@ -146,27 +146,27 @@ describe('error-router', function() { var err = {}; var MESSAGE = 'test'; err = {message: MESSAGE}; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.message, MESSAGE); }); it('Message Field: Should default the field given lack-of input', function() { var err = {}; - errorHandlerRouter(err, em); - assert.strictEqual(em.message, ''); + populateErrorMessage(err, em); + assert(em.message.startsWith('[object Object]')); }); it('User field: Should write to the field given valid input', function() { var err = {}; var USER = 'test'; err.user = USER; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.user, USER); }); it('User field: Should default the field given lack-of input', function() { var err = {}; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.user, ''); }); @@ -174,13 +174,13 @@ describe('error-router', function() { var err = {}; var PATH = 'test'; err.filePath = PATH; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.filePath, PATH); }); it('FilePath Field: Should default the field given lack-of input', function() { var err = {}; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.filePath, ''); }); @@ -188,13 +188,13 @@ describe('error-router', function() { var err = {}; var LINE_NUMBER = 10; err.lineNumber = LINE_NUMBER; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.lineNumber, LINE_NUMBER); }); it('LineNumber Field: Should default the field given lack-of input', function() { var err = {}; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.lineNumber, 0); }); @@ -202,13 +202,13 @@ describe('error-router', function() { var err = {}; var FUNCTION_NAME = 'test'; err.functionName = FUNCTION_NAME; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.functionName, FUNCTION_NAME); }); it('FunctionName Field: Should default the field given lack-of input', function() { var err = {}; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.functionName, ''); }); @@ -216,7 +216,7 @@ describe('error-router', function() { var err = {}; var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; err.serviceContext = TEST_SERVICE_VALID; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.serviceContext, TEST_SERVICE_VALID); }); @@ -224,13 +224,13 @@ describe('error-router', function() { var err = {}; var TEST_SERVICE_INVALID = 12; err.serviceContext = TEST_SERVICE_INVALID; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); }); it('ServiceContext Field: Should default the field given lack-of input', function() { var err = {}; - errorHandlerRouter(err, em); + populateErrorMessage(err, em); assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); }); }); From 8034df564b943f2dd16d963476090aadccda9ec7 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 7 Jun 2017 00:32:30 -0700 Subject: [PATCH 06/11] Rename error-router to populate-error-message --- packages/error-reporting/src/interfaces/express.js | 2 +- packages/error-reporting/src/interfaces/hapi.js | 2 +- packages/error-reporting/src/interfaces/koa.js | 2 +- packages/error-reporting/src/interfaces/manual.js | 2 +- packages/error-reporting/src/interfaces/restify.js | 2 +- .../src/{error-router.js => populate-error-message.js} | 0 .../test/unit/{error-router.js => populate-error-message.js} | 4 ++-- 7 files changed, 7 insertions(+), 7 deletions(-) rename packages/error-reporting/src/{error-router.js => populate-error-message.js} (100%) rename packages/error-reporting/test/unit/{error-router.js => populate-error-message.js} (98%) diff --git a/packages/error-reporting/src/interfaces/express.js b/packages/error-reporting/src/interfaces/express.js index 4779be205e1..4a1df34366e 100644 --- a/packages/error-reporting/src/interfaces/express.js +++ b/packages/error-reporting/src/interfaces/express.js @@ -21,7 +21,7 @@ var isFunction = is.fn; var ErrorMessage = require('../classes/error-message.js'); var expressRequestInformationExtractor = require('../request-extractors/express.js'); -var populateErrorMessage = require('../error-router.js'); +var populateErrorMessage = require('../populate-error-message.js'); /** * Returns a function that can be used as an express error handling middleware. diff --git a/packages/error-reporting/src/interfaces/hapi.js b/packages/error-reporting/src/interfaces/hapi.js index 18d8c4291c6..de904d08158 100644 --- a/packages/error-reporting/src/interfaces/hapi.js +++ b/packages/error-reporting/src/interfaces/hapi.js @@ -20,7 +20,7 @@ var isObject = is.object; var isFunction = is.fn; var ErrorMessage = require('../classes/error-message.js'); var hapiRequestInformationExtractor = require('../request-extractors/hapi.js'); -var populateErrorMessage = require('../error-router.js'); +var populateErrorMessage = require('../populate-error-message.js'); var packageJson = require('../../package.json'); /** diff --git a/packages/error-reporting/src/interfaces/koa.js b/packages/error-reporting/src/interfaces/koa.js index 450da5d267a..8d9357a6697 100644 --- a/packages/error-reporting/src/interfaces/koa.js +++ b/packages/error-reporting/src/interfaces/koa.js @@ -17,7 +17,7 @@ 'use strict'; var ErrorMessage = require('../classes/error-message.js'); var koaRequestInformationExtractor = require('../request-extractors/koa.js'); -var populateErrorMessage = require('../error-router.js'); +var populateErrorMessage = require('../populate-error-message.js'); /** * The koaErrorHandler should be placed at the beginning of the koa middleware diff --git a/packages/error-reporting/src/interfaces/manual.js b/packages/error-reporting/src/interfaces/manual.js index 6d686ce849d..4e85e247e2a 100644 --- a/packages/error-reporting/src/interfaces/manual.js +++ b/packages/error-reporting/src/interfaces/manual.js @@ -22,7 +22,7 @@ var isFunction = is.fn; var ErrorMessage = require('../classes/error-message.js'); var manualRequestInformationExtractor = require('../request-extractors/manual.js'); -var populateErrorMessage = require('../error-router.js'); +var populateErrorMessage = require('../populate-error-message.js'); /** * The handler setup function serves to produce a bound instance of the diff --git a/packages/error-reporting/src/interfaces/restify.js b/packages/error-reporting/src/interfaces/restify.js index 3ff4ac072aa..1fa0ce40af9 100644 --- a/packages/error-reporting/src/interfaces/restify.js +++ b/packages/error-reporting/src/interfaces/restify.js @@ -21,7 +21,7 @@ var isFunction = is.function; var ErrorMessage = require('../classes/error-message.js'); var expressRequestInformationExtractor = require('../request-extractors/express.js'); -var populateErrorMessage = require('../error-router.js'); +var populateErrorMessage = require('../populate-error-message.js'); /** * The restifyErrorHandler is responsible for taking the captured error, setting diff --git a/packages/error-reporting/src/error-router.js b/packages/error-reporting/src/populate-error-message.js similarity index 100% rename from packages/error-reporting/src/error-router.js rename to packages/error-reporting/src/populate-error-message.js diff --git a/packages/error-reporting/test/unit/error-router.js b/packages/error-reporting/test/unit/populate-error-message.js similarity index 98% rename from packages/error-reporting/test/unit/error-router.js rename to packages/error-reporting/test/unit/populate-error-message.js index dac601db8be..1cbf96723d7 100644 --- a/packages/error-reporting/test/unit/error-router.js +++ b/packages/error-reporting/test/unit/populate-error-message.js @@ -19,7 +19,7 @@ var assert = require('assert'); var ErrorMessage = require('../../src/classes/error-message.js'); -var populateErrorMessage = require('../../src/error-router.js'); +var populateErrorMessage = require('../../src/populate-error-message.js'); var TEST_USER_INVALID = 12; var TEST_MESSAGE = 'This is a test'; @@ -30,7 +30,7 @@ var TEST_STACK_DEFAULT = { functionName: '' }; -describe('error-router', function() { +describe('populate-error-message', function() { var em; var adversarialObjectInput = { stack: {} From cf81ad3bb941188271b26947cdf6116843f3ff9e Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 7 Jun 2017 09:56:51 -0700 Subject: [PATCH 07/11] Reorganize the populate-error-message tests --- .../test/unit/populate-error-message.js | 149 ++++++++++-------- 1 file changed, 85 insertions(+), 64 deletions(-) diff --git a/packages/error-reporting/test/unit/populate-error-message.js b/packages/error-reporting/test/unit/populate-error-message.js index 1cbf96723d7..79386c9ed0e 100644 --- a/packages/error-reporting/test/unit/populate-error-message.js +++ b/packages/error-reporting/test/unit/populate-error-message.js @@ -77,7 +77,8 @@ describe('populate-error-message', function() { populateErrorMessage.bind(null, adversarialObjectInputTwo, em)); }); - it('Message field: Should set the message as the stack of the given error', function() { + it('Message Field: Should set the message as the stack given an Error', + function() { var err = new Error(TEST_MESSAGE); populateErrorMessage(err, em); assert.deepEqual(em.message, err.stack, 'Given a valid message the ' + @@ -85,7 +86,24 @@ describe('populate-error-message', function() { ); }); - it('User field: Should set the user field if given valid input', function() { + it('Message Field: Should set the field given valid input given an object', + function() { + var err = {}; + var MESSAGE = 'test'; + err = {message: MESSAGE}; + populateErrorMessage(err, em); + assert.strictEqual(em.message, MESSAGE); + }); + + it('Message Field: Should default the field given lack-of input given ' + + 'an object', function() { + var err = {}; + populateErrorMessage(err, em); + assert(em.message.startsWith('[object Object]')); + }); + + it('User Field: Should set the field given valid input given an Error', + function() { var err = new Error(); var TEST_USER_VALID = 'TEST_USER'; err.user = TEST_USER_VALID; @@ -93,14 +111,32 @@ describe('populate-error-message', function() { assert.strictEqual(em.context.user, TEST_USER_VALID); }); - it('User field: Should default the user field if given invalid input', function() { + it('User Field: Should default the field given invalid input given an Error', + function() { var err = new Error(); err.user = TEST_USER_INVALID; populateErrorMessage(err, em); assert.strictEqual(em.context.user, ''); }); - it('Service field: Should set the field if given valid input', function() { + it('User Field: Should set the field given valid input given an object', + function() { + var err = {}; + var USER = 'test'; + err.user = USER; + populateErrorMessage(err, em); + assert.strictEqual(em.context.user, USER); + }); + + it('User Field: Should default the field given lack-of input given an ' + + 'object', function() { + var err = {}; + populateErrorMessage(err, em); + assert.strictEqual(em.context.user, ''); + }); + + it('ServiceContext Field: Should set the field given valid input given ' + + 'an Error', function() { var err = new Error(); var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; err.serviceContext = TEST_SERVICE_VALID; @@ -108,7 +144,8 @@ describe('populate-error-message', function() { assert.deepEqual(err.serviceContext, TEST_SERVICE_VALID); }); - it('Service field: Should default the field if given invalid input', function() { + it('ServiceContext Field: Should default the field given invalid input ' + + 'given an Error', function() { var err = new Error(); var TEST_SERVICE_INVALID = 12; err.serviceContext = TEST_SERVICE_INVALID; @@ -116,13 +153,40 @@ describe('populate-error-message', function() { assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); }); - it('Service field: Should default the field if not given input', function() { + it('ServiceContext Field: Should default the field if not given input ' + + 'given an Error', function() { var err = new Error(); populateErrorMessage(err, em); assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); }); - it('Report location field: Should default the field if given invalid input', function() { + it('ServiceContext Field: Should set the field given valid input given an ' + + 'object', function() { + var err = {}; + var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; + err.serviceContext = TEST_SERVICE_VALID; + populateErrorMessage(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_VALID); + }); + + it('ServiceContext Field: Should default the field given invalid input ' + + 'given an object', function() { + var err = {}; + var TEST_SERVICE_INVALID = 12; + err.serviceContext = TEST_SERVICE_INVALID; + populateErrorMessage(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); + }); + + it('ServiceContext Field: Should default the field given lack-of input ' + + 'given an object', function() { + var err = {}; + populateErrorMessage(err, em); + assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); + }); + + it('Report location Field: Should default the field if given invalid input ' + + 'given an Error', function() { var TEST_STACK_INVALID_CONTENTS = { filePath: null, lineNumber: '2', @@ -134,7 +198,8 @@ describe('populate-error-message', function() { assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); }); - it('Report location field: Should default field if not given a valid type', function() { + it('Report location Field: Should default field if not given a valid type ' + + 'given an Error', function() { var err = new Error(); var TEST_STACK_INVALID_TYPE = []; err.stack = TEST_STACK_INVALID_TYPE; @@ -142,35 +207,8 @@ describe('populate-error-message', function() { assert.deepEqual(em.context.reportLocation, TEST_STACK_DEFAULT); }); - it('Message Field: Should write to the field given valid input', function() { - var err = {}; - var MESSAGE = 'test'; - err = {message: MESSAGE}; - populateErrorMessage(err, em); - assert.strictEqual(em.message, MESSAGE); - }); - - it('Message Field: Should default the field given lack-of input', function() { - var err = {}; - populateErrorMessage(err, em); - assert(em.message.startsWith('[object Object]')); - }); - - it('User field: Should write to the field given valid input', function() { - var err = {}; - var USER = 'test'; - err.user = USER; - populateErrorMessage(err, em); - assert.strictEqual(em.context.user, USER); - }); - - it('User field: Should default the field given lack-of input', function() { - var err = {}; - populateErrorMessage(err, em); - assert.strictEqual(em.context.user, ''); - }); - - it('FilePath Field: Should write to the field given valid input', function() { + it('FilePath Field: Should set the field given valid input given an object', + function() { var err = {}; var PATH = 'test'; err.filePath = PATH; @@ -178,13 +216,15 @@ describe('populate-error-message', function() { assert.strictEqual(em.context.reportLocation.filePath, PATH); }); - it('FilePath Field: Should default the field given lack-of input', function() { + it('FilePath Field: Should default the field given lack-of input given ' + + 'an object', function() { var err = {}; populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.filePath, ''); }); - it('LineNumber Field: Should write to the field given valid input', function() { + it('LineNumber Field: Should set the field given valid input given an object', + function() { var err = {}; var LINE_NUMBER = 10; err.lineNumber = LINE_NUMBER; @@ -192,13 +232,15 @@ describe('populate-error-message', function() { assert.strictEqual(em.context.reportLocation.lineNumber, LINE_NUMBER); }); - it('LineNumber Field: Should default the field given lack-of input', function() { + it('LineNumber Field: Should default the field given lack-of input given ' + + 'an object', function() { var err = {}; populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.lineNumber, 0); }); - it('FunctionName Field: Should write to the field given valid input', function() { + it('FunctionName Field: Should set the field given valid input given ' + + 'an object', function() { var err = {}; var FUNCTION_NAME = 'test'; err.functionName = FUNCTION_NAME; @@ -206,31 +248,10 @@ describe('populate-error-message', function() { assert.strictEqual(em.context.reportLocation.functionName, FUNCTION_NAME); }); - it('FunctionName Field: Should default the field given lack-of input', function() { + it('FunctionName Field: Should default the field given lack-of input given ' + + 'an object', function() { var err = {}; populateErrorMessage(err, em); assert.strictEqual(em.context.reportLocation.functionName, ''); }); - - it('ServiceContext Field: Should write to the field given valid input', function() { - var err = {}; - var TEST_SERVICE_VALID = {service: 'test', version: 'test'}; - err.serviceContext = TEST_SERVICE_VALID; - populateErrorMessage(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_VALID); - }); - - it('ServiceContext Field: Should default the field given invalid input', function() { - var err = {}; - var TEST_SERVICE_INVALID = 12; - err.serviceContext = TEST_SERVICE_INVALID; - populateErrorMessage(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); - }); - - it('ServiceContext Field: Should default the field given lack-of input', function() { - var err = {}; - populateErrorMessage(err, em); - assert.deepEqual(em.serviceContext, TEST_SERVICE_DEFAULT); - }); }); From b747c3b88d5223be9b3996feec34463aa8d6deeb Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Tue, 13 Jun 2017 14:52:39 -0700 Subject: [PATCH 08/11] Update buildStackTrace to use captureStackTrace The `Error.captureStackTrace` method is now used in the `buildStackTrace` function to create a stack trace. --- .../error-reporting/src/build-stack-trace.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/error-reporting/src/build-stack-trace.js b/packages/error-reporting/src/build-stack-trace.js index cd9571dc403..17507493bbf 100644 --- a/packages/error-reporting/src/build-stack-trace.js +++ b/packages/error-reporting/src/build-stack-trace.js @@ -22,7 +22,6 @@ var is = require('is'); * Constructs a string representation of the stack trace at the point where * this function was invoked. Note that the stack trace will not include any * references to this function itself. - * @function buildStackTrace * @param {String?} message - The message that should appear as the first line * of the stack trace. This value defaults to the empty string. * @param {Number?} numSkip - The number of additional lines to not display @@ -33,13 +32,16 @@ var is = require('is'); * where this method was invoked. */ function buildStackTrace(message, numSkip) { - var fauxError = new Error(''); - var fullStack = fauxError.stack.split('\n'); + var target = {}; + // Build a stack trace without the frames associated with `buildStackTrace`. + // The stack is located at `target.stack`. + Error.captureStackTrace(target, buildStackTrace); + // Ignore the first line, which contains the error message, because we will + // add our own error message later. + var fullStack = target.stack.split('\n').slice(1); var prefix = message ? message + '\n' : ''; - // Two additional lines are skipped to skip the default empty message - // and the line referencing this function itself. - var totalNumSkip = (is.number(numSkip) ? numSkip : 0) + 2; - return prefix + fullStack.slice(totalNumSkip).join('\n'); + var realNumSkip = is.number(numSkip) ? numSkip : 0; + return prefix + fullStack.slice(realNumSkip).join('\n'); } module.exports = buildStackTrace; From 26d98ab80c5508c6765018eea1a8c4c1743f309a Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Tue, 13 Jun 2017 16:43:36 -0700 Subject: [PATCH 09/11] Simplify the system tests The code for verifiying that stack traces don't contain error-reporting specific frames has been consolidated into a single location. --- .../system-test/testAuthClient.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/error-reporting/system-test/testAuthClient.js b/packages/error-reporting/system-test/testAuthClient.js index 087b1b7180c..d29f00e8b2b 100644 --- a/packages/error-reporting/system-test/testAuthClient.js +++ b/packages/error-reporting/system-test/testAuthClient.js @@ -446,6 +446,10 @@ describe('error-reporting', function() { assert.equal(errItem.count, 1); var rep = errItem.representative; assert.ok(rep); + // Ensure the stack trace in the message does not contain frames + // specific to the error-reporting library. The reportManualError + // method is the entry point for reporting errors. + assert.strictEqual(rep.message.indexOf('reportManualError'), -1); var context = rep.serviceContext; assert.ok(context); assert.strictEqual(context.service, SERVICE); @@ -475,8 +479,7 @@ describe('error-reporting', function() { var errorId = buildName('with-error-constructor'); var errOb = new Error(errorId); verifyReporting(errOb, function(message) { - return message.startsWith('Error: ' + errorId + - '\n at Context.verifyErrors'); + return message.startsWith('Error: ' + errorId + '\n'); }, TIMEOUT, done); }); @@ -484,21 +487,21 @@ describe('error-reporting', function() { this.timeout(TIMEOUT * 2); var errorId = buildName('with-string'); verifyReporting(errorId, function(message) { - return message.startsWith(errorId + '\n at verifyReporting'); + return message.startsWith(errorId + '\n'); }, TIMEOUT, done); }); it('Should correctly publish an error that is undefined', function(done) { this.timeout(TIMEOUT * 2); verifyReporting(undefined, function(message) { - return message.startsWith('undefined\n at verifyReporting'); + return message.startsWith('undefined\n'); }, TIMEOUT, done); }); it('Should correctly publish an error that is null', function(done) { this.timeout(TIMEOUT * 2); verifyReporting(null, function(message) { - return message.startsWith('null\n at verifyReporting'); + return message.startsWith('null\n'); }, TIMEOUT, done); }); @@ -506,7 +509,7 @@ describe('error-reporting', function() { function(done) { this.timeout(TIMEOUT * 2); verifyReporting({ someKey: 'someValue' }, function(message) { - return message.startsWith('[object Object]\n at verifyReporting'); + return message.startsWith('[object Object]\n'); }, TIMEOUT, done); }); @@ -514,7 +517,7 @@ describe('error-reporting', function() { this.timeout(TIMEOUT * 2); var num = (new Date()).getTime(); verifyReporting(num, function(message) { - return message.startsWith('' + num + '\n at verifyReporting'); + return message.startsWith('' + num + '\n'); }, TIMEOUT, done); }); @@ -523,7 +526,7 @@ describe('error-reporting', function() { this.timeout(TIMEOUT * 2); var bool = true; verifyReporting(bool, function(message) { - return message.startsWith('true\n at verifyReporting'); + return message.startsWith('true\n'); }, TIMEOUT, done); }); From 3df370b98ebc651e5bb8c0ad83b0ffc60a1006f2 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Tue, 13 Jun 2017 17:09:54 -0700 Subject: [PATCH 10/11] Consolidate code in populate-error-message.js --- .../src/populate-error-message.js | 60 +------------------ 1 file changed, 3 insertions(+), 57 deletions(-) diff --git a/packages/error-reporting/src/populate-error-message.js b/packages/error-reporting/src/populate-error-message.js index bf3ad343803..c3e36d12e2c 100644 --- a/packages/error-reporting/src/populate-error-message.js +++ b/packages/error-reporting/src/populate-error-message.js @@ -18,9 +18,6 @@ var has = require('lodash.has'); var is = require('is'); var isObject = is.object; -var isString = is.string; -var isNumber = is.number; -var isFunction = is.function; var buildStackTrace = require('./build-stack-trace.js'); /** @@ -35,17 +32,13 @@ var buildStackTrace = require('./build-stack-trace.js'); */ function populateErrorMessage(ob, em) { if (ob === null || ob === undefined) { - populateFromUnknown(ob, em); - } else if (ob instanceof Error) { + em.setMessage(buildStackTrace('' + ob, 3)); + } else if (ob.stack) { populateFromError(ob, em); } else if (typeof ob === 'object' && isObject(ob)) { populateFromObject(ob, em); - } else if (typeof ob === 'string' && isString(ob)) { - populateFromString(ob, em); - } else if (typeof ob === 'number') { - populateFromNumber(ob, em); } else { - populateFromUnknown(ob, em); + em.setMessage(buildStackTrace(ob.toString(), 3)); } return em; @@ -129,51 +122,4 @@ function populateFromObject(ob, errorMessage) { } } -/** - * Handles validation of an error which has been indicated to be of type String. - * This function will create a new instance of the Error class to produce a - * stack trace for submission to the API and check to confirm that the given - * value is of type string. - * @function populateFromString - * @param {String} str - the String indicated as the content of the error - * @param {ErrorMessage} errorMessage - the error message instance to marshal - * error information into. - * @returns {Undefined} - does not return anything - */ -function populateFromString(str, errorMessage) { - errorMessage.setMessage(buildStackTrace(str, 3)); -} - -/** - * Handles routing and validation for parsing an error which has been indicated - * to be of type Number. This handler will manufacture a new Error to create - * a stack-trace for submission to the Error API and will attempt to caste the - * given number to a string for submission to the Error API. - * @function populateFromNumber - * @param {Number} num - the number submitted as content for the error message - * @param {ErrorMessage} errorMessage - the error messag instance to marshall - * error information into. - * @returns {Undefined} - does not return anything - */ -function populateFromNumber(num, errorMessage) { - var message = isNumber(num) && isFunction(num.toString) ? num.toString() : ''; - errorMessage.setMessage(buildStackTrace(message, 3)); -} - -/** - * Handles unknown/unsupported input as the content of the error message. Since - * the problem-space is not defined for this path the library only attempts to - * manufacture a stack trace for submission to the API and discards the input - * that was given as the error content. - * @function populateFromUnknown - * @param {Any} ob - the unknown/unsupported input indicated as the content of - * the error. - * @param {ErrorMessage} errorMessage - the error message instance to marshal - * error information into. - * @returns {Undefined} - does not return anything - */ -function populateFromUnknown(ob, errorMessage) { - errorMessage.setMessage(buildStackTrace('' + ob, 3)); -} - module.exports = populateErrorMessage; From 24fbc3b8f63f64ac5ffdebefc37825b0cfc2447d Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Thu, 15 Jun 2017 15:10:13 -0700 Subject: [PATCH 11/11] Make the `buildStackTrace` more robust In particular, the `buildStackTrace` was updated to ensure that no error-reporting specific frames are included in the built stack trace. In addition, the system-tests have been updated to ensure error-reporting related frames, and only those frames, are removed from the built stack trace. --- .../error-reporting/src/build-stack-trace.js | 19 ++---- .../src/interfaces/message-builder.js | 2 +- .../src/populate-error-message.js | 6 +- .../system-test/testAuthClient.js | 66 ++++++++++++++----- .../test/unit/build-stack-trace.js | 48 ++++---------- 5 files changed, 70 insertions(+), 71 deletions(-) diff --git a/packages/error-reporting/src/build-stack-trace.js b/packages/error-reporting/src/build-stack-trace.js index 17507493bbf..f90f56b7fc1 100644 --- a/packages/error-reporting/src/build-stack-trace.js +++ b/packages/error-reporting/src/build-stack-trace.js @@ -16,32 +16,27 @@ 'use strict'; -var is = require('is'); +const SRC_ROOT = __dirname; /** * Constructs a string representation of the stack trace at the point where * this function was invoked. Note that the stack trace will not include any - * references to this function itself. + * references to frames specific to this error-reporting library itself. * @param {String?} message - The message that should appear as the first line * of the stack trace. This value defaults to the empty string. - * @param {Number?} numSkip - The number of additional lines to not display - * in the stack trace. That is, the stack trace returned will have the - * specified message with the top `numSkip` lines of the stack not displayed. - * This value defaults to 0 if it is not defined or is not a number. * @returns {String} - A string representation of the stack trace at the point * where this method was invoked. */ -function buildStackTrace(message, numSkip) { +function buildStackTrace(message) { var target = {}; // Build a stack trace without the frames associated with `buildStackTrace`. // The stack is located at `target.stack`. Error.captureStackTrace(target, buildStackTrace); - // Ignore the first line, which contains the error message, because we will - // add our own error message later. - var fullStack = target.stack.split('\n').slice(1); var prefix = message ? message + '\n' : ''; - var realNumSkip = is.number(numSkip) ? numSkip : 0; - return prefix + fullStack.slice(realNumSkip).join('\n'); + return prefix + target.stack.split('\n').slice(1).filter(function(line) { + // Filter out all frames that are specific to the error-reporting library + return !line || line.indexOf(SRC_ROOT) === -1; + }).join('\n'); } module.exports = buildStackTrace; diff --git a/packages/error-reporting/src/interfaces/message-builder.js b/packages/error-reporting/src/interfaces/message-builder.js index 6cd4e470eb8..03c7aefd746 100644 --- a/packages/error-reporting/src/interfaces/message-builder.js +++ b/packages/error-reporting/src/interfaces/message-builder.js @@ -45,7 +45,7 @@ function handlerSetup(config) { // error is used instead of the stack trace where the error is // reported to be consistent with the behavior of reporting a // an error when reporting an actual Node.js Error object. - var cleanedStack = buildStackTrace('', 1); + var cleanedStack = buildStackTrace(''); var em = new ErrorMessage().setServiceContext( config.getServiceContext().service, diff --git a/packages/error-reporting/src/populate-error-message.js b/packages/error-reporting/src/populate-error-message.js index c3e36d12e2c..a082dce5afa 100644 --- a/packages/error-reporting/src/populate-error-message.js +++ b/packages/error-reporting/src/populate-error-message.js @@ -32,13 +32,13 @@ var buildStackTrace = require('./build-stack-trace.js'); */ function populateErrorMessage(ob, em) { if (ob === null || ob === undefined) { - em.setMessage(buildStackTrace('' + ob, 3)); + em.setMessage(buildStackTrace('' + ob)); } else if (ob.stack) { populateFromError(ob, em); } else if (typeof ob === 'object' && isObject(ob)) { populateFromObject(ob, em); } else { - em.setMessage(buildStackTrace(ob.toString(), 3)); + em.setMessage(buildStackTrace(ob.toString())); } return em; @@ -97,7 +97,7 @@ function populateFromObject(ob, errorMessage) { if (has(ob, 'message')) { errorMessage.setMessage(ob.message); } else { - errorMessage.setMessage(buildStackTrace('' + ob, 3)); + errorMessage.setMessage(buildStackTrace('' + ob)); } if (has(ob, 'user')) { diff --git a/packages/error-reporting/system-test/testAuthClient.js b/packages/error-reporting/system-test/testAuthClient.js index d29f00e8b2b..251e5cf4ff7 100644 --- a/packages/error-reporting/system-test/testAuthClient.js +++ b/packages/error-reporting/system-test/testAuthClient.js @@ -32,6 +32,7 @@ var assign = require('lodash.assign'); var pick = require('lodash.pick'); var omitBy = require('lodash.omitby'); var util = require('util'); +var path = require('path'); const ERR_TOKEN = '_@google_STACKDRIVER_INTEGRATION_TEST_ERROR__'; const TIMEOUT = 30000; @@ -362,9 +363,11 @@ describe('Expected Behavior', function() { }); describe('error-reporting', function() { + const SRC_ROOT = path.join(__dirname, '..', 'src'); + const TIMESTAMP = Date.now(); const BASE_NAME = 'error-reporting-system-test'; function buildName(suffix) { - return [Date.now(), BASE_NAME, suffix].join('_'); + return [TIMESTAMP, BASE_NAME, suffix].join('_'); } const SERVICE = buildName('service-name'); @@ -446,10 +449,16 @@ describe('error-reporting', function() { assert.equal(errItem.count, 1); var rep = errItem.representative; assert.ok(rep); - // Ensure the stack trace in the message does not contain frames - // specific to the error-reporting library. The reportManualError - // method is the entry point for reporting errors. - assert.strictEqual(rep.message.indexOf('reportManualError'), -1); + // Ensure the stack trace in the message does not contain any frames + // specific to the error-reporting library. + assert.strictEqual(rep.message.indexOf(SRC_ROOT), -1); + // Ensure the stack trace in the mssage contains the frame corresponding + // to the 'expectedTopOfStack' function because that is the name of + // function used in this file that is the topmost function in the call + // stack that is not internal to the error-reporting library. + // This ensures that only the frames specific to the + // error-reporting library are removed from the stack trace. + assert.notStrictEqual(rep.message.indexOf('expectedTopOfStack'), -1); var context = rep.serviceContext; assert.ok(context); assert.strictEqual(context.service, SERVICE); @@ -459,12 +468,14 @@ describe('error-reporting', function() { } function verifyReporting(errOb, messageTest, timeout, cb) { - errors.report(errOb, function(err, response, body) { - assert.ifError(err); - assert(isObject(response)); - assert.deepEqual(body, {}); - verifyServerResponse(messageTest, timeout, cb); - }); + (function expectedTopOfStack() { + errors.report(errOb, function(err, response, body) { + assert.ifError(err); + assert(isObject(response)); + assert.deepEqual(body, {}); + verifyServerResponse(messageTest, timeout, cb); + }); + })(); } // For each test below, after an error is reported, the test waits @@ -477,7 +488,9 @@ describe('error-reporting', function() { function verifyErrors(done) { this.timeout(TIMEOUT * 2); var errorId = buildName('with-error-constructor'); - var errOb = new Error(errorId); + var errOb = (function expectedTopOfStack() { + return new Error(errorId); + })(); verifyReporting(errOb, function(message) { return message.startsWith('Error: ' + errorId + '\n'); }, TIMEOUT, done); @@ -533,9 +546,17 @@ describe('error-reporting', function() { it('Should correctly publish errors using an error builder', function(done) { this.timeout(TIMEOUT * 2); var errorId = buildName('with-error-builder'); + // Use an IIFE with the name `definitionSiteFunction` to use later to ensure + // the stack trace of the point where the error message was constructed is + // used. + // Use an IIFE with the name `expectedTopOfStack` so that the test can + // verify that the stack trace used does not contain any frames + // specific to the error-reporting library. var errOb = (function definitionSiteFunction() { - return errors.event() - .setMessage(errorId); + return (function expectedTopOfStack() { + return errors.event() + .setMessage(errorId); + })(); })(); (function callingSiteFunction() { verifyReporting(errOb, function(message) { @@ -554,15 +575,22 @@ describe('error-reporting', function() { this.timeout(TIMEOUT * 2); reinitialize({ reportUnhandledRejections: true }); var rejectValue = buildName('promise-rejection'); - Promise.reject(rejectValue); + (function expectedTopOfStack() { + // An Error is used for the rejection value so that it's stack + // contains the stack trace at the point the rejection occured and is + // rejected within a function named `expectedTopOfStack` so that the + // test can verify that the collected stack is correct. + Promise.reject(new Error(rejectValue)); + })(); + var rejectText = 'Error: ' + rejectValue; setImmediate(function() { var expected = 'UnhandledPromiseRejectionWarning: Unhandled ' + - 'promise rejection: ' + rejectValue + + 'promise rejection: ' + rejectText + '. This rejection has been reported to the ' + 'Google Cloud Platform error-reporting console.'; assert.notStrictEqual(logOutput.indexOf(expected), -1); verifyServerResponse(function(message) { - return message.startsWith(rejectValue); + return message.startsWith(rejectText); }, TIMEOUT, done); }); }); @@ -571,7 +599,9 @@ describe('error-reporting', function() { this.timeout(TIMEOUT * 2); reinitialize({ reportUnhandledRejections: false }); var rejectValue = buildName('promise-rejection'); - Promise.reject(rejectValue); + (function expectedTopOfStack() { + Promise.reject(rejectValue); + })(); setImmediate(function() { var notExpected = 'UnhandledPromiseRejectionWarning: Unhandled ' + 'promise rejection: ' + rejectValue + diff --git a/packages/error-reporting/test/unit/build-stack-trace.js b/packages/error-reporting/test/unit/build-stack-trace.js index c3706ad56d4..f12c313934a 100644 --- a/packages/error-reporting/test/unit/build-stack-trace.js +++ b/packages/error-reporting/test/unit/build-stack-trace.js @@ -17,69 +17,43 @@ 'use strict'; var assert = require('assert'); +var path = require('path'); var buildStackTrace = require('../../src/build-stack-trace.js'); +const SRC_ROOT = path.join(__dirname, '..', '..', 'src'); + describe('build-stack-trace', function() { - it('Should not have a message attached if none is given without specifying to skip frames', function() { + it('Should not have a message attached if none is given', function() { assert(buildStackTrace().startsWith(' at')); assert(!buildStackTrace(undefined).startsWith('undefined')); assert(!buildStackTrace(null).startsWith('null')); }); - it('Should not have a message attached if none is given with specifying to skip frames', function() { - assert(buildStackTrace().startsWith(' at'), 2); - assert(!buildStackTrace(undefined).startsWith('undefined'), 2); - assert(!buildStackTrace(null).startsWith('null'), 2); - }); - - it('Should attach a message if given without specifying to skip frames', function() { + it('Should attach a message if given', function() { assert(buildStackTrace('Some Message').startsWith('Some Message\n')); }); - it('Should attach a message if given with specifying to skip frames', function() { - assert(buildStackTrace('Some Message').startsWith('Some Message\n'), 2); - }); - - it('Should not contain error-reporting specific frames without specifying to skip frames', function() { - var internalFileName = 'build-stack-trace'; - var stackTrace = buildStackTrace(); - var firstIndex = stackTrace.indexOf(internalFileName); - var lastIndex = stackTrace.lastIndexOf(internalFileName); - // This file, named 'build-stack-trace.js', tests the - // 'build-stack-trace.js' file. The stack trace should not contain - // information about the 'build-stack-trace.js' file that is being - // tested. Thus the stack trace should only contain the string - // 'build-stack-trace' one time, which corresponds to this test file - // and not the 'build-stack-trace.js' file being tested. - assert.strictEqual(firstIndex, lastIndex); - }); - - it('Should return the stack trace without specifying to skip frames', function() { + it('Should not contain error-reporting specific frames', function() { (function functionA() { (function functionB() { (function functionC() { var stackTrace = buildStackTrace(); assert(stackTrace); - assert.notStrictEqual(stackTrace.indexOf('functionA'), -1); - assert.notStrictEqual(stackTrace.indexOf('functionB'), -1); - assert.notStrictEqual(stackTrace.indexOf('functionC'), -1); + assert.strictEqual(stackTrace.indexOf(SRC_ROOT), -1); })(); })(); })(); }); - it('Should return the stack trace with the correct number of frames skipped', function() { + it('Should return the stack trace', function() { (function functionA() { (function functionB() { (function functionC() { - var stackTrace = buildStackTrace('', 2); + var stackTrace = buildStackTrace(); assert(stackTrace); - // The stack trace should not contain the frames for - // functionC or functionB because 2 frames were specified as being - // skipped. It should, however, contain functionA. assert.notStrictEqual(stackTrace.indexOf('functionA'), -1); - assert.strictEqual(stackTrace.indexOf('functionB'), -1); - assert.strictEqual(stackTrace.indexOf('functionC'), -1); + assert.notStrictEqual(stackTrace.indexOf('functionB'), -1); + assert.notStrictEqual(stackTrace.indexOf('functionC'), -1); })(); })(); })();