nest
nest copied to clipboard
Allow `app.select` to not abort on error
Is there an existing issue that is already proposing this?
- [X] I have searched the existing issues
Is your feature request related to a problem? Please describe it
When using app.select to get a module than have been loaded dynamically if the module is not found the app will crash and it's not possible to catch the error to prevent the crash.
Describe the solution you'd like
Add a new boolean arg abortOnError to app.select that allow to not "abort on error" and catch the error.
By default abortOnError will be true to keep the current behavior.
Cf this issue for more details: https://github.com/nestjs/nest/issues/13033
Teachability, documentation, adoption, migration strategy
Users can pass the new arg to false to disable the "abort" behavior.
With true as default value there are no retro compatibility problems.
What is the motivation / use case for changing the behavior?
I have some modules that are loaded dynamically based on different parameters/conditions. But some modules will need some specific operations. So I select a module, if it's loaded I will run the specific operations, if it's not I will ignore it and do nothing. Right now if the module is not loaded the app crash.
By default
abortOnErrorwill betrueto keep the current behavior.
I'd say that we should align it with the abortOnError of NestFactory instead.
So if we supply abortOnError: true to NestFactory, app.select(Bar) will use abortOnError:true as well, which is the current behavior.
If we supply abortOnError:false to NestFactory, app.select(Bar) will use abortOnError: false as well.
Then we can fine-tuning each app.select call if we want to by calling app.select(Bar, { abortOnError: true })
I believe this is better than having to coordinate the two behaviors in several places (imagine using app.select in multiple places). Although could be confusing if we have app.select(Bar, { abortOnError: true }) and app.select(Qux) because it won't be that clear what is the default for the latter
What do you think?
Yes I think this is better.
and for completeness, I'd say that we should support the same for app.get() errors as well. As of now, we cannot handle exceptions on them when having abortOnError:true (the default)
Let's track this here https://github.com/nestjs/nest/pull/14113