search

jeffijoe / awilix-express

Awilix helpers/middleware for Express
MIT License
118 stars 7 forks source link

Handle errors in async routes #15

Closed elunic closed 5 years ago

elunic commented 5 years ago

We've been using bottlejs extensively but are looking to switch to Awilix to leverage its advanced capabilities when it comes to scoped and transient service instances as well as awilix-express for the decorator syntax.

We've written our own adapter for express and bottlejs (bottlejs-express). One thing we included that is immensely useful is handling of errors in routes that are async functions. When an async method (route) throws an error, that rejected promise is not handled by express or awilix-express.

As a result, each method would have to include a try/catch block, which is easy to forget, resulting in swallowed errors, or greatly reduces readability of the code.

The included PR introduces a wrapper for handling rejected promises coming from async routes. I have included as many tests as I thought were relevant. We're really looking forward to using this module, but it's not an option without this error handling.

Please let me know if there are any adjustments you think should still be made. Thanks!

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 5e6e7bb54460f57876c96428dad29d818251b7a4 on elunic:feature/async-routes-error-handling into ff3afebf230f402d72a79fb9263143297429841e on talyssonoc:master.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 5e6e7bb54460f57876c96428dad29d818251b7a4 on elunic:feature/async-routes-error-handling into ff3afebf230f402d72a79fb9263143297429841e on talyssonoc:master.

jeffijoe commented 5 years ago

I think this is related to #12 in that @chanlito wants to be able to catch errors promise-style.

Looks good to me!

@talyssonoc: I think if you merge and ship this, you can close #12 as well and have no more open issues on the repo.

P.S. @elunic thank you for maintaining 100% code coverage! šŸ™Œ

elunic commented 5 years ago

No problem. Nobody wants their PR to break a repo!

maxstrauch commented 5 years ago

I really like this feature and would appreciate if it is merged very soon. šŸ‘

@talyssonoc could you merge it soon? (or: @jhoscar1 or @blove)

jeffijoe commented 5 years ago

I've reached out to @talyssonoc on Twitter to see if he would consider giving me contributing and publishing rights.

jeffijoe commented 5 years ago

@elunic @whefter @maxstrauch This has been released as v2.0.0 on npm ā€” the major increment is due to the Awilix v3 -> v4 update.

Thanks for the PR!

maxstrauch commented 5 years ago

Thanks for merging in šŸ‘ šŸ˜„