Add next to default oauth handlers

[heyitsaamir]

Without these, we don't propagate the activity to any other handlers

[aacebo]

the intension is that when an error occurs that isn't caught in an activity handler, the execution of all handlers coming after should not take place. Handling of said error should be done through the error event handler.

[aacebo]

is the use case that you want to implement custom handlers for the auth invoke activities? since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them? I do think we need to have a way to disable the default auth invoke handlers to allow for custom implementations

[heyitsaamir]

is the use case that you want to implement custom handlers for the auth invoke activities? since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them? I do think we need to have a way to disable the default auth invoke handlers to allow for custom implementations

Ya just for these.

since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them

It's just unexpected that my handlers just don't work if i have them in app.on('signin.verifystate') etc. I could just wanna log it? Isn't it just good practice to propagate it? We can do it on success instead of always if that's the concern?

[singhk97]

is the use case that you want to implement custom handlers for the auth invoke activities? since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them? I do think we need to have a way to disable the default auth invoke handlers to allow for custom implementations

Ya just for these.

since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them

It's just unexpected that my handlers just don't work if i have them in app.on('signin.verifystate') etc. I could just wanna log it? Isn't it just good practice to propagate it? We can do it on success instead of always if that's the concern?

I believe the convention is that when there's an error in the middleware it should terminate processing. Middlewares can determine when and if the next handler should be called.

I agree that it's confusing if the user defined singin.verifyState handler is not triggered in a non-error scenario. It's worth adding a comment in the method definiton labelling the auth handlers as having default implementation. And these handlers could also have an "override" param if that makes sense w/ the existing architecture.

[heyitsaamir]

@aacebo @singhk97 updated, take a look :)

[aacebo]

is the use case that you want to implement custom handlers for the auth invoke activities? since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them? I do think we need to have a way to disable the default auth invoke handlers to allow for custom implementations

Ya just for these.

since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them

It's just unexpected that my handlers just don't work if i have them in app.on('signin.verifystate') etc. I could just wanna log it? Isn't it just good practice to propagate it? We can do it on success instead of always if that's the concern?

oh I see, on that case I would personally expect that if you do app.on('signin.verify-state', ...) our default handler implementation should be replaced entirely, I think we should add an optional replace = false to the app.on method that when true replaces all existing routes of that selector with the provided new route, this would allow devs to override our defaults safely.

[heyitsaamir]

is the use case that you want to implement custom handlers for the auth invoke activities? since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them? I do think we need to have a way to disable the default auth invoke handlers to allow for custom implementations

Ya just for these.

since this would only be an issue with verify state or exchange token im wondering what handlers you would need to come after them

It's just unexpected that my handlers just don't work if i have them in app.on('signin.verifystate') etc. I could just wanna log it? Isn't it just good practice to propagate it? We can do it on success instead of always if that's the concern?

oh I see, on that case I would personally expect that if you do app.on('signin.verify-state', ...) our default handler implementation should be replaced entirely, I think we should add an optional replace = false to the app.on method that when true replaces all existing routes of that selector with the provided new route, this would allow devs to override our defaults safely.

Ah that's an interesting idea. Though, then the devs would need to be aware that those routes require replacing right? Or are you saying for some routes (like the ones we provide default handlers for), we always replace the default handler? We could always flag "isInternal" for a given handler if we write it internally, and if the application overrides it, we replace it?