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

Improve Jinja Template feedback and error handling #884

Merged
merged 20 commits into from
Nov 28, 2022

Conversation

mderynck
Copy link
Contributor

@mderynck mderynck commented Nov 22, 2022

What this PR does:
When editing jinja templates for alert presentation or outgoing webhooks the UI was returning vague errors back to the console making it difficult for the user to correct their template. Also if errors were encountered during runtime exceptions were being raised which blocked the alert from being displayed in the UI. This PR improves that experience.

General Template Changes

  • Templates are limited in size (50000 characters), larger templates will result in JinjaTemplateError
  • Rendered template results are truncated to a size limit (50000 characters)
  • range function can no longer be used
  • All errors in compiling and running templates are categorized as JinjaTemplateError or JinjaTemplateWarning
    • JinjaTemplateError should block saving of the template
    • JinjaTemplateWarning should be displayed but not block saving. The purpose of this is when templates are saved we don't always have a complete representation of the data that will be used, this allows for accessing fields in the data which may be present at runtime but are not present when the template is being written.
  • Any template resulting in a SecurityError is logged

Alert Group Template Editor

  • Preview area now displays errors/warnings to assist in fixing the template
  • On pressing Save Templates a notification will popup if any errors block saving (Errors will block saving, Warnings will not)
    • In some cases errors are allowed to be saved due to the template code causing the error to not execute in all cases (inside a conditional)

Alert Groups

  • Titles rendered in templates are truncated to a size limit (500 characters)
  • Each alert body is truncated to a size limit (50000 characters), this also improves performance for both rendering and processing as it avoids urlize needing to process as much data (https://github.com/grafana/oncall-private/issues/1445)
  • If processing an alert template results in an error/warning it will be displayed as the content instead of raising an exception.

Outgoing Webhooks

  • Errors/Warnings display in a notification popup to better assist correcting templates.
  • Validation of the result from the Data template has been relaxed.
  • If processing the Data template results in an error/warning it will be delivered as the content of the webhook instead of raising an exception.

Which issue(s) this PR fixes:
#890
https://github.com/grafana/oncall-private/issues/1446
https://github.com/grafana/oncall-private/issues/1445
https://github.com/grafana/oncall-private/issues/1431

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated

@mderynck mderynck marked this pull request as ready for review November 24, 2022 19:40
Copy link
Contributor

@matiasb matiasb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mderynck mderynck merged commit 3582f9b into dev Nov 28, 2022
@mderynck mderynck deleted the mderynck/jinja-error-feedback-improvement branch November 28, 2022 16:46
brojd pushed a commit that referenced this pull request Sep 18, 2024
* Improve feedback so template errors are given to user

* Add security error logging

* Add limits for templates, payloads, results

* Show popup error notification for webhook errors and template errors that don't have a result

* Update tests

* Split exceptions into warnings/errors to give more control when previewing, rendering, saving templates

* Limit title lengths

* Make TypeError a warning

* Adjust title length limit

* Remove length limiting on urlize since it is being done on template render

* Fix tests

* Add KeyError and ValueError to warnings

* No longer enforcing json result when saving webhook in case it is dependent on payload

* Add tests for expected exceptions coming from apply_jinja_template

* Update changelog

* Send raw post if template result is not JSON
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants