Skip to content

Commit

Permalink
fix(error-logging): split up logging and displaying
Browse files Browse the repository at this point in the history 
also improves naming and one line error logging output
  • Loading branch information
Benedikt Rötsch committed Jul 14, 2017
1 parent fd84ba6 commit 09b5783
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 77 deletions.
2 changes: 0 additions & 2 deletions lib/get/get-outdated-destination-content.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ export default function getOutdatedDestinationContent ({managementClient, spaceI
locales: skipContentModel ? Promise.resolve([]) : space.getLocales().then(extractItems),
webhooks: []
})
}, (err) => {
throw err
})
}

Expand Down
4 changes: 2 additions & 2 deletions lib/push/publishing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ export function publishEntities (entities) {
})

return Promise.map(entitiesToPublish, (entity, index) => {
logEmitter.emit('info', `Publishing ${entity.sys.type} ${getEntityName(entity)}`)
return entity.publish()
.then((entity) => {
logEmitter.emit('info', `Published ${entity.sys.type} ${getEntityName(entity)}`)
return entity
}, (err) => {
logEmitter.emit('error', err)
Expand All @@ -38,7 +38,7 @@ export function publishEntities (entities) {
.then((entity) => Promise.delay(500, entity))
}, {concurrency: 1})
.then((result) => {
logEmitter.emit('info', `Finished publishing ${entities.length} ${type}s. Returning ${result.length} entities`)
logEmitter.emit('info', `Successfully published ${result.length} ${type}s.`)
return result
})
}
120 changes: 69 additions & 51 deletions lib/utils/logging.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { isObject } from 'lodash'
import figures from 'figures'
import moment from 'moment'

import getEntityName from './get-entity-name'

export const logEmitter = new EventEmitter()

function extractErrorInformation (error) {
Expand All @@ -15,78 +17,90 @@ function extractErrorInformation (error) {
throw new Error('could not extract API error data')
}

export function formatErrorOneLine (error) {
if (error.hasOwnProperty('warning')) {
return error.warning
export function formatLogMessageOneLine (logMessage) {
const { level } = logMessage
if (!level) {
return logMessage.toString().replace(/\s+/g, ' ')
}
if (level === 'info') {
return logMessage.info
}
if (level === 'warning') {
return logMessage.warning
}
try {
// Display enhanced API error message when available
const errorOutput = []
const data = extractErrorInformation(error.error)
if ('entity' in data) {
errorOutput.push(`Entity ID: ${data.entity.sys.id || 'unknown'}`)
}
if ('requestId' in data) {
errorOutput.push(`Request ID: ${data.requestId}`)
}
const data = extractErrorInformation(logMessage.error)
if ('status' in data || 'statusText' in data) {
const status = [data.status, data.statusText].filter((a) => a).join(' - ')
errorOutput.push(`Status: ${status}`)
}
if ('message' in data) {
errorOutput.push(`Message: ${data.message}`)
}
if ('entity' in data) {
errorOutput.push(`Entity: ${getEntityName(data.entity)}`)
}
if ('details' in data && 'errors' in data.details) {
const errorList = data.details.errors.map((error) => error.details || error.name)
errorOutput.push(`Details: ${errorList.join(', ')}`)
}
return `${error.error.name}: ${errorOutput.join(' - ')}`
} catch (err) {
if (!error.hasOwnProperty(('error'))) {
error = {error}
if ('requestId' in data) {
errorOutput.push(`Request ID: ${data.requestId}`)
}
return error.error.toString().replace(/[\n\r]/, ' ')
return `${logMessage.error.name}: ${errorOutput.join(' - ')}`
} catch (err) {
// Fallback for errors without API information
return logMessage.error.toString().replace(/\s+/g, ' ')
}
}

export function formatErrorLogfile (error) {
if (error.hasOwnProperty('warning')) {
return error
export function formatLogMessageLogfile (logMessage) {
const { level } = logMessage
if (level === 'info' || level === 'warning') {
return logMessage
}
if (!logMessage.error) {
// Enhance node errors to logMessage format
logMessage.error = logMessage
}
try {
const data = extractErrorInformation(error.error)
const errorOutput = Object.assign({}, error.error, {data})
// Enhance error with extracted API error log
const data = extractErrorInformation(logMessage.error)
const errorOutput = Object.assign({}, logMessage.error, {data})
delete errorOutput.message
error.error = errorOutput
return error
logMessage.error = errorOutput
} catch (err) {
if (error.error.stack) {
error.error.stacktrace = error.error.stack.toString().split(/\n +at /)
// Fallback for errors without API information
if (logMessage.error.stack) {
logMessage.error.stacktrace = logMessage.error.stack.toString().split(/\n +at /)
}
return error
}
return logMessage
}

// Display all errors
export function displayErrorLog (errorLog) {
if (errorLog.length) {
const warningsCount = errorLog.filter((error) => error.hasOwnProperty('warning')).length
const errorsCount = errorLog.filter((error) => error.hasOwnProperty('error')).length
console.log(`\n\nThe following ${errorsCount} errors and ${warningsCount} warnings occurred:\n`)

errorLog.map((error) => {
if (!error.hasOwnProperty('ts')) {
return formatErrorOneLine({error})
}
return `${moment(error.ts).format('HH:mm:SS')} - ${formatErrorOneLine(error)}`
}).map((error) => console.log(error))
errorLog
.map((logMessage) => `${moment(logMessage.ts).format('HH:mm:SS')} - ${formatLogMessageOneLine(logMessage)}`)
.map((logMessage) => console.log(logMessage))

return
}
console.log('No errors or warnings occurred')
}

// Write all log messages instead of infos to the error log file
export function writeErrorLogFile (destination, errorLog) {
const logFileErrors = errorLog.map(formatErrorLogfile)
return bfj.write(destination, logFileErrors, {
const logFileData = errorLog
.map(formatLogMessageLogfile)
return bfj.write(destination, logFileData, {
circular: 'ignore',
space: 2
})
Expand All @@ -102,36 +116,40 @@ export function writeErrorLogFile (destination, errorLog) {
})
}

export function setupErrorLogging (errorLog) {
// Init listeners for log messages, transform them into proper format and logs/displays them
export function setupLogging (log) {
function errorLogger (level, error) {
errorLog.push({
const logMessage = {
ts: (new Date()).toJSON(),
level,
[level]: error
})
}
if (level !== 'info') {
log.push(logMessage)
}
logEmitter.emit('display', logMessage)
}

logEmitter.addListener('warning', (error) => errorLogger('warning', error))
logEmitter.addListener('error', (error) => errorLogger('error', error))
logEmitter.addListener('info', (error) => errorLogger('info', error))
}

// Format log message to display them as task status
export function logToTaskOutput (task) {
function logInfo (info) {
task.output = `${figures.tick} ${info}`
}
function logWarning (warning) {
task.output = `${figures.warning} ${warning}`
}
function logError (error) {
const formattedError = formatErrorOneLine(error)
task.output = `${figures.cross} ${formattedError}`
function logToTask (logMessage) {
const content = formatLogMessageOneLine(logMessage)
const symbols = {
'info': figures.tick,
'warning': figures.warning,
'error': figures.cross
}
task.output = `${symbols[logMessage.level]} ${content}`.trim()
}
logEmitter.on('info', logInfo)
logEmitter.on('warning', logWarning)
logEmitter.on('error', logError)

logEmitter.on('display', logToTask)

return () => {
logEmitter.removeListener('info', logInfo)
logEmitter.removeListener('warning', logWarning)
logEmitter.removeListener('error', logError)
logEmitter.removeListener('display', logToTask)
}
}
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"bfj-node4": "4.1.0",
"bluebird": "^3.3.3",
"contentful": "^4.5.0",
"contentful-management": "^3.8.0",
"contentful-management": "^3.9.1",
"figures": "^2.0.0",
"listr": "^0.12.0",
"listr-verbose-renderer": "^0.4.0",
Expand Down
6 changes: 3 additions & 3 deletions test/push/publishing-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ test('Only publishes valid entities and does not fail when api error occur', (t)
])
.then((result) => {
t.equals(publishStub.callCount, 2, 'tries to publish both assets, while skipping the faulty asset')
t.equals(fakeLogEmitter.emit.args[4][0], 'error', 'logs error at correct point of time')
t.equals(fakeLogEmitter.emit.args[4][1], errorValidation, 'logs correct error')
t.equals(fakeLogEmitter.emit.callCount, 6, 'should log start, end, unparseable notice, one success message and one error')
t.equals(fakeLogEmitter.emit.args[5][0], 'error', 'logs error at correct point of time')
t.equals(fakeLogEmitter.emit.args[5][1], errorValidation, 'logs correct error')
t.equals(fakeLogEmitter.emit.callCount, 7, 'should log start, end, unparseable notice, one success message and one error')
t.equals(result.length, 2, 'Result only contains resolved & valid entities')
teardown()
t.end()
Expand Down
30 changes: 12 additions & 18 deletions test/utils/logging-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import test from 'tape'

import {
formatErrorOneLine,
formatErrorLogfile
formatLogMessageOneLine,
formatLogMessageLogfile
} from '../../lib/utils/logging'

test('format one line api error', (t) => {
Expand All @@ -24,26 +24,20 @@ test('format one line api error', (t) => {
}
const json = JSON.stringify(apiError)
const error = new Error(json)
const output = formatErrorOneLine({error})
t.equals(output, 'Error: Entity ID: 42 - Request ID: 3 - Message: Some API error - Details: error detail')
const output = formatLogMessageOneLine({error, level: 'error'})
t.equals(output, 'Error: Message: Some API error - Entity: 42 - Details: error detail - Request ID: 3')
t.end()
})

test('format one line invalid api error', (t) => {
const invalidApiError = {
message: 'invalid api error',
entity: {}
}
const json = JSON.stringify(invalidApiError)
const error = new Error(json)
const output = formatErrorOneLine({error})
t.equals(output, `Error: ${json}`)
test('format one line standard error', (t) => {
const output = formatLogMessageOneLine({error: Error('normal error message'), level: 'error'})
t.equals(output, 'Error: normal error message')
t.end()
})

test('format one line standard error', (t) => {
const output = formatErrorOneLine({error: Error('normal error message')})
t.equals(output, 'Error: normal error message')
test('format one line standard warning', (t) => {
const output = formatLogMessageOneLine({warning: 'warning text', level: 'warning'})
t.equals(output, 'warning text')
t.end()
})

Expand All @@ -66,7 +60,7 @@ test('format log file api error', (t) => {
}
const json = JSON.stringify(apiError)
const error = new Error(json)
const output = formatErrorLogfile({error})
const output = formatLogMessageLogfile({error, level: 'error'})
t.equals(output.error.data.requestId, apiError.requestId)
t.equals(output.error.data.message, apiError.message)
t.equals(output.error.data.details.errors[0].name, apiError.details.errors[0].name)
Expand All @@ -75,7 +69,7 @@ test('format log file api error', (t) => {

test('format log file standard error', (t) => {
const error = new Error('normal error message')
const output = formatErrorLogfile({error})
const output = formatLogMessageLogfile({error, level: 'error'})
t.equals(output.error.message, error.message)
t.end()
})

0 comments on commit 09b5783

Please sign in to comment.