Skip to content

Commit

Permalink
catch potential write errors on fatal flush sync (#740)
Browse files Browse the repository at this point in the history
* Add forceFlushOnFatal flag

* Remove unused line

* Do nothing on error

* Update lib/levels.js

Co-Authored-By: David Mark Clements <david.mark.clements@gmail.com>

* Remove once
  • Loading branch information
allevo authored and mcollina committed Nov 18, 2019
1 parent 0416d88 commit 919f5ad
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/levels.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ const levelMethods = {
const stream = this[streamSym]
logFatal.call(this, ...args)
if (typeof stream.flushSync === 'function') {
stream.flushSync()
try {
stream.flushSync()
} catch (e) {
// https://github.com/pinojs/pino/pull/740#discussion_r346788313
}
}
},
error: genLog(levels.error),
Expand Down
13 changes: 13 additions & 0 deletions test/levels.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,16 @@ test('fatal method sync-flushes the destination if sync flushing is available',
instance.fatal('this is fatal')
})
})

test('fatal method should call async when sync-flushing fails', ({ equal, fail, doesNotThrow, plan }) => {
plan(2)
const messages = [
'this is fatal 1'
]
const stream = sink((result) => equal(result.msg, messages.shift()))
stream.flushSync = () => { throw new Error('Error') }
stream.flush = () => fail('flush should be called')

const instance = pino(stream)
doesNotThrow(() => instance.fatal(messages[0]))
})

0 comments on commit 919f5ad

Please sign in to comment.