Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement em::findOneOrFail #133

Closed
kgoncharov opened this issue Sep 19, 2019 · 7 comments
Closed

Implement em::findOneOrFail #133

kgoncharov opened this issue Sep 19, 2019 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@kgoncharov
Copy link

Is your feature request related to a problem? Please describe.
Sometimes it's very convenient to have a short method that will fail-fast if nothing was found.

Describe the solution you'd like
em::findOrFail that return Promise<T> or throw exception if entity wasn't found.

@kgoncharov kgoncharov added the enhancement New feature or request label Sep 19, 2019
@B4nan
Copy link
Member

B4nan commented Sep 19, 2019

Agreed, I am usually defining this method in custom base repository, but sure we can have it integrated as well. Personally I am using custom NotFoundException that is later on catched in main request handler and handled as 404 response. I guess we could allow configuring a callback where users could provide their own exception and possibly error message based on what was not found.

Maybe better name would be findOneOrFail? or even findOneOrThrow?

@B4nan B4nan added this to the 3.0 milestone Sep 19, 2019
@kgoncharov
Copy link
Author

I supposed that this method would be used only for situations, where the entity should be in database, and if there is no entity server should return InternalServerException at the end, to show that there's some inconsistency in database.
For this purposes findOneOrFail name would be the best.
On the other hand there could be two methods:
findOneOrThrow - for custom handling on what exception we're going to throw (NotFoundException, etc)
findOneOrFail - for fatal and critical errors that will return some critical custom exception that could be propagated to InternalServerException
@B4nan wdyt?

@B4nan
Copy link
Member

B4nan commented Sep 19, 2019

I think that could cause a lot of confusion, I would like to have only one method for this.

Btw for your InternalServerException use case, how are you handling 404s? I guess manually, right? Then with my proposal you would just set up the callback to produce InternalServerException and keep handling the 404 manually (either via try/catch or via truthy checks). Or as I already said, you can always define your own methods in base repository (you can set your own base repository for all entities in ORM's configuration).

@kgoncharov
Copy link
Author

Yep, manually.
Anyway, just findOneOrFail with capability to set custom exception to throw would be great.

@vimmerru
Copy link

vimmerru commented Sep 20, 2019

I would like

cons used = await findOneOrFail(
    id,
    () => InternalServerError(`No expected user in db: ${id}`));

@B4nan
Copy link
Member

B4nan commented Sep 20, 2019

findOne already has more complex interface:

async findOne<T extends IEntityType<T>>(entityName: EntityName<T>, where: FilterQuery<T> | IPrimaryKey, populate?: string[] | FindOneOptions, orderBy?: QueryOrderMap): Promise<T | null> {

But yes, I like the idea to allow overriding the default exception handler via it's parameters, probably via FindOneOptions (probably FindOneOrFailOptions that will extend that).

@B4nan
Copy link
Member

B4nan commented Sep 20, 2019

Available for testing in 3.0.0-alpha.9. Closing as resolved.

Docs: 0d57b7b#diff-474106f26d234ca3b44599c73da3d296R112

@B4nan B4nan closed this as completed Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants