-
Notifications
You must be signed in to change notification settings - Fork 20
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
fix: webhook ui failure handling #374
Conversation
@@ -429,7 +429,7 @@ where | |||
enqueue_alert(error_msg.clone(), AlertType::Error, 5000); | |||
return Err(error_msg); | |||
} | |||
if status.is_server_error() { | |||
if status.is_server_error() && status != 512 { |
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.
Why are we ignoring this very specific error code?
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.
This is a custom error code sent by the webhook.
When the webhook fails, the steps involving the experiment and CAC are already completed. Users receive a false signal that the experiment wasn't created, prompting them to attempt creation multiple times, potentially leading to duplicate experiments.
why is this pointed to main ? |
saas might take a while to go to prod |
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.
We shouldn't ignore the webhook failure going forward. Rather this should be displayed in the experiments screen otherwise the user assumes even the webhook trigger succeeded.
Problem
Webhook failures in frontend not handled properly
Solution
added a condition to ignore the 512 status code