-
Notifications
You must be signed in to change notification settings - Fork 3
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
🥅 Handle template errors #20
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Evaline Ju <[email protected]>
Signed-off-by: Evaline Ju <[email protected]>
Signed-off-by: Evaline Ju <[email protected]>
# but the error message may not be directly user-comprehensible | ||
# This has to be caught first because jinja UndefinedError are TemplateError | ||
chat_response = ErrorResponse( | ||
message="Template error. Please check request.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the message in this case not usable to be passed to the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - an example is example 2 of the PR description e.g. 'dict object' has no attribute 'prompt'
when prompt
is not an end-user-facing parameter. I think considerations could be made to see if some more of these can be given as feedback to be caught as exceptions in the default template [depending on the model].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. true. but that information might still be helpful for debugging. So can we log the raw error message while we switch it with better error message here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, we can just fall through to the general TemplateError
then
Signed-off-by: Evaline Ju <[email protected]>
c9a90f6
to
722c6a5
Compare
Catch template errors so that the server does not just
Internal Server Error
on fixable requests. The assumption here is that since the (chat) template can be overwritten, errors can be considered bad requests.Here, some particular error cases are addressed -
TemplateError
are returned fromraise_exception
in jinja templates.UndefinedError
are a sub-class ofTemplateError
.UndefinedError
may not give easily user-fixable messages, but these are propagated anyway, since it was decided that such improvements can be made at the template (or default model chat template) level.Testing
Example error calls with a granite guardian model server
Example 1 - wrong "role" ("moo" is not an expected "role" for the default risk
harm
)Before this update, the call would
Internal Server Error
. In logs one could seejinja2.exceptions.TemplateError: [role, risk] combination is incorrect
. Now{"object":"error","message":"[role, risk] combination is incorrect","type":"BadRequestError","param":null,"code":400}%
Example 2 - wrong number of messages for the relevance risk
Before this update, the call would
Internal Server Error
. In logs one could seejinja2.exceptions.UndefinedError: 'dict object' has no attribute 'prompt'
. Now{"object":"error","message":"'dict object' has no attribute 'prompt'","type":"BadRequestError","param":null,"code":400}%
Closes: #14