feat: catch errors of wrong use of handlebars #5527
Conversation
✅ Deploy Preview for novu-design ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ainouzgali
requested review from
a team,
davidsoderberg and
antonjoel82
and removed request for
a team
| const template = Handlebars.compile(templateContent); | ||
|
|
||
| result = template(command.data, {}); | ||
| } catch (e: any) { |
There was a problem hiding this comment.
| } catch (e: any) { | |
| } catch (e: unknown) { |
Better TS ;)
There was a problem hiding this comment.
Unfortunately, it creates a ts error for e.message 😬
There was a problem hiding this comment.
At the moment, thats how all catch errors are typed in the project
There was a problem hiding this comment.
@ainouzgali You can use checkIsResponseError as a type-guard here! TypeScript began recommending unknown as the type for catch in TS 4.4
There was a problem hiding this comment.
@BiswaViraj Side note: I would suggest that we reserve Request changes for blocking problems (like issue or important chores) 🙂 -- this is probably more of a nitpick until we put it into a coding style guide
(But agree with commenting on it so we have fewer places to change later 🙌 )
There was a problem hiding this comment.
Cool. Didn't know about it. Thanks!
…compile' into nv-3772-missing-catch-for-email-compile
ainouzgali
dismissed
BiswaViraj’s stale review
I really want to merge it before the next scheduled tests ❤️
* feat: catch errors of wrong use of handlebars * feat: use type guard for error
What changed? Why was the change needed?
For compile email template, there was no catch block for wrong use of handlebars syntax which resulted in
I added the try-catch on the actual template compilation - so we won't miss adding it in each place we are using that usecase.
Sentry links:
1, 2, 3, 4, 5, 6
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer