Error propagation

[StarpTech]

What's the reason to prevent the error propagation here?

The user is not able to catch the error in the ready event after a ready or after callback was registered.

const boot = require('./')

function Server () {}
const app = boot(new Server())

function plugin (server, opts, done) {
  done(new Error('test'))
}

app.use(plugin)
.after((err) => console.error('Plugin: ' + err))

app.ready(function (err) {
  console.error('Ready: ' + err)
})

// Plugin: Error: test
// Ready: null

[delvedor]

If you want to propagate the error just use the next callback of after.

const boot = require('./')

function Server () {}
const app = boot(new Server())

function plugin (server, opts, done) {
  done(new Error('test'))
}

app.use(plugin)
.after((err, next) => next(err))

app.ready(function (err) {
  console.error('Ready: ' + err)
})

[StarpTech]

I thought that was intentional :) but avvio has the goal to make bootstrapping simple. This is a great place for user issues like in fastify. In my opinion, this should be the task of the bootstraper because an error is a signal for a bad circumstance of your application and it shouldn't exist a path to escape.

[delvedor]

The idea of after is to handle the error just after its happened, in this way you can have a second path to execute in case of error.
If you propagate it by default you cannot do that.

Is very hard to decide what to do in this edge situations. Probably document this should be the best thing to do. :)

[StarpTech]

It's not possible to pass the last error but allow an override ?

next(null) // reset explicit the previous error
next() // no error or inherit the last error

with this solution, we would follow an error first approach.

[mcollina]

IMHO we can change this here. I'm kind of stuck atm, if anyone of you wants to take the lead I'd be very happy.

[StarpTech]

@mcollina WDYT about this ticket?

[mcollina]

I think the function in after should not be triggered with the error.

[mcollina]

it's a semver-major change, though.

[StarpTech]

I would change it but it's also fine for me when we find a way to document it clearly.

[delvedor]

I think the function in after should not be triggered with the error.

I think this is too rigid.
Take the following:

const boot = require('./')

function Server () {}
const app = boot(new Server())

function plugin (server, opts, done) {
    done(new Error('oh no!'))
}

function backup (server, opts, done) {
   done()
}

app.use(plugin)
  .after((err, next) => {
    if (err) {
       app.use(backup)
    }
    next()
  })
)

app.ready(function (err) {
  console.error('Ready: ' + err)
})

This could be a nice pattern, otherwise the use of after is not that useful.

[mcollina]

let's see with the fix in avvio, and then we can decide.

[mcollina]

Is this still relevant? I'm closing, as I hopefully should be working correctly now.