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

FailureMode propagation and handling in services #149

Closed
adam-cattermole opened this issue Nov 20, 2024 · 2 comments · Fixed by #158
Closed

FailureMode propagation and handling in services #149

adam-cattermole opened this issue Nov 20, 2024 · 2 comments · Fixed by #158

Comments

@adam-cattermole
Copy link
Member

There is a flaw in the logic to the handling of errors for each of the services - we should always propagate the error to the caller for the next action in the set.

Example; if there's two services defined, the first action being FailureMode::Allow followed by FailureMode::Deny and for any reason we have an issue in the process_grpc_response function for the first service we call handle_error_on_grpc_response which will resume_http_request ignoring the subsequent actions effectively allowing the request through.

This error block for example will also be hit if we have a timeout on our grpc call to the initial service (service code 14 with an empty message body).

@eguzki
Copy link
Contributor

eguzki commented Nov 21, 2024

This error block for example will also be hit if we have a timeout on our grpc call to the initial service (service code 14 with an empty message body).

That's because kuadrant's module does not check status_code in

fn on_grpc_call_response(&mut self, token_id: u32, status_code: u32, resp_size: usize) {

The code should never reach process_grpc_response when status_code is other than ''OK'' (I think it is ''0'').

@adam-cattermole
Copy link
Member Author

adam-cattermole commented Nov 21, 2024

That's an option, but we also need to think about what the different return codes might mean for the expected outcome and whether the failure mode makes sense in each case - whether we can/should continue with subsequent actions or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
2 participants