onError hook is called before errorHandler when error is thrown in onRequest hook

[jillro]

onError hook is called before errorHandler when error is thrown in onRequest hook

[mcollina]

Was it working in some cases before this? Or has it always been buggy?

[mcollina]

A PR with a fix would really be nice.

[Eomm]

Can replicate with:

const t = require('tap')
const test = t.test
const Fastify = require('.')

test('Fastify should throw on wrong options', async t => {
  const app = Fastify()

  app.addHook('onError', async (req, res, error) => {
    t.pass('onError called')
  })

  app.setErrorHandler((error, req, res) => {
    t.pass('errorHandler called')
    if (error.message === 'unauthorized') {
      return res.code(401).send('unauthorized')
    }

    throw error
  })

  app.get('/should_not_be_logged', {
    onRequest: async () => {
      throw new Error('unauthorized')
    }
  }, async (req, res) => {
    res.send('OK')
  })

  app.get('/should_be_logged', {
    onRequest: async () => {
      throw new Error('unhandled error')
    }
  }, async (req, res) => {
    res.send('OK')
  })

  await app.ready()

  const res = await app.inject({
    method: 'GET',
    url: '/should_not_be_logged'
  })
  t.same(res.statusCode, 401)

  const res2 = await app.inject({
    method: 'GET',
    url: '/should_be_logged'
  })
  t.same(res2.statusCode, 500)

  t.end()
})

We should check.

In v3 I get:

# Subtest: Fastify should throw on wrong options
    ok 1 - errorHandler called
    ok 2 - should be equivalent
    ok 3 - errorHandler called
    ok 4 - onError called
    ok 5 - should be equivalent
    1..5
ok 1 - Fastify should throw on wrong options # time=55.848ms

In v4.10.0:

# Subtest: Fastify should throw on wrong options
    ok 1 - onError called
    ok 2 - errorHandler called
    ok 3 - should be equivalent
    ok 4 - onError called
    ok 5 - errorHandler called
    not ok 6 - unhandled error // memory error

It may me think if we have outdated docs, but for sure we must add a test to assert the order