Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle non-Error objects thrown as error gracefully, still attempt to report them in a useful way #232

Merged
merged 5 commits into from Mar 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 52 additions & 22 deletions src/features/notify.ts
Expand Up @@ -56,26 +56,50 @@ export class NotifyFeature implements Feature {
this.logger('destroy')
}

notifyError (err: Error, context?: ErrorContext) {
getSafeError (err): Error {
if (err instanceof Error) return err

let message: string
try {
message = `Non-error value: ${JSON.stringify(err)}`
} catch (e) {
// We might land here if the error value was not serializable (it might contain
// circular references for example), or if user code was ran as part of the
// serialization and that code threw an error.
// As alternative, we can try converting the error to a string instead:
try {
message = `Unserializable non-error value: ${String(e)}`
// Intentionally not logging the second error anywhere, because we would need
// to protect against errors while doing *that* as well. (For example, the second
// error's "toString" property may crash or not exist.)
} catch (e2) {
// That didn't work. So, report a totally unknown error that resists being converted
// into any usable form.
// Again, we don't even attempt to look at that third error for the same reason as
// described above.
message = `Unserializable non-error value that cannot be converted to a string`
}
}
if (message.length > 1000) message = message.substr(0, 1000) + '...'

return new Error(message)
}

notifyError (err: Error | string | {}, context?: ErrorContext) {
// set default context
if (typeof context !== 'object') {
context = { }
}

if (!(err instanceof Error)) {
console.error('You must use notifyError with an Error object, ignoring')
console.trace()
return -1
}

if (this.transport === undefined) {
return this.logger(`Tried to send error without having transporter available`)
}

const safeError = this.getSafeError(err)
const payload = {
message: err.message,
stack: err.stack,
name: err.name,
message: safeError.message,
stack: safeError.stack,
name: safeError.name,
metadata: context
}

Expand All @@ -89,10 +113,12 @@ export class NotifyFeature implements Feature {
console.error(error)
}

const safeError = this.getSafeError(error)

const payload = {
message: error.message,
stack: error.stack,
name: error.name
message: safeError.message,
stack: safeError.stack,
name: safeError.name
}

if (ServiceManager.get('transport')) {
Expand All @@ -107,10 +133,12 @@ export class NotifyFeature implements Feature {

console.error(error)

const safeError = this.getSafeError(error)

const payload = {
message: error.message,
stack: error.stack,
name: error.name
message: safeError.message,
stack: safeError.stack,
name: safeError.name
}

if (ServiceManager.get('transport')) {
Expand All @@ -132,10 +160,11 @@ export class NotifyFeature implements Feature {
error : true
})
return function errorHandler (err, req, res, next) {
const safeError = this.getSafeError(err)
const payload = {
message: err.message,
stack: err.stack,
name: err.name,
message: safeError.message,
stack: safeError.stack,
name: safeError.name,
metadata: {
http: {
url: req.url,
Expand Down Expand Up @@ -167,10 +196,11 @@ export class NotifyFeature implements Feature {
try {
await next()
} catch (err) {
const safeError = this.getSafeError(err)
const payload = {
message: err.message,
stack: err.stack,
name: err.name,
message: safeError.message,
stack: safeError.stack,
name: safeError.name,
metadata: {
http: {
url: ctx.request.url,
Expand Down
2 changes: 1 addition & 1 deletion src/pmx.ts
Expand Up @@ -176,7 +176,7 @@ export default class PMX {
* Notify an error to PM2 Plus/Enterprise, note that you can attach a context to it
* to provide more insight about the error
*/
notifyError (error: Error, context?: ErrorContext) {
notifyError (error: Error | string | {}, context?: ErrorContext) {
const notify = this.featureManager.get('notify') as NotifyFeature
return notify.notifyError(error, context)
}
Expand Down
6 changes: 5 additions & 1 deletion test/fixtures/apiNotifyChild.ts
Expand Up @@ -2,4 +2,8 @@
import * as pmx from '../../src/index'

pmx.init()
pmx.notifyError(new Error('myNotify'))
try {
throw new Error('myNotify')
} catch (err) {
pmx.notifyError(err)
}