Skip to content

Conversation

maherthomsi
Copy link
Contributor

@maherthomsi maherthomsi commented Sep 2, 2025

Issue number:
#774

Description of changes:
Fix a bug where an index out-of-bounds error would be returned when "Status" is not in the apiclient response error message. The previous string-based approach using "Status" as a delimiter could fail if the delimiter was not present or formatted differently.

This change replaces string manipulation with proper HTTP status code parsing using the http crate's StatusCode type. The extraction now safely handles missing delimiters and invalid status codes by returning the StatusCode instead of raw string slices.

Testing done:
Ran make build

Validated status code parsing function locally.

Created unit tests for status code parsing function

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@ytsssun ytsssun left a comment

Choose a reason for hiding this comment

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

Thanks for this fix, can we try to add unit tests for the index out of bound issue?

Also were you able to reproduce the original issue? If yes, it would also be a good idea to run the "integration test" to confirm the fix.

fn extract_status_code_from_error(error: &str) -> &str {
let error_content_split_by_status: Vec<&str> = error.split("Status").collect();
let error_content_split_by_whitespace: Vec<&str> = error_content_split_by_status[1]
fn extract_status_code_from_error(error: &str) -> Option<StatusCode> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there is a more direct way of getting that status code such as directly use the unix socket and invoke the APIServer using an http client.

Having to parse the status code from the response feels a bit odd to me..

Copy link
Contributor Author

@maherthomsi maherthomsi Sep 9, 2025

Choose a reason for hiding this comment

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

We shouldn't directly use the unix socket. The intended architectural approach is to utilize the existing apiclient binary rather than creating a custom HTTP client to communicate directly with the Bottlerocket API. If we do want this functionality, the change would take place in the apiclient rather than within Brupop, and would be tied to the newer Bottlerocket releases. Since the API client would return the response rather than just the code, we would need to parse the response for the code.

@maherthomsi
Copy link
Contributor Author

I was not able to reproduce the original issue since there was a custom build involved, but I was able to include unit tests to mock the scenario experienced in the original issue to validate that the fix works.

@maherthomsi maherthomsi requested a review from ytsssun September 9, 2025 22:48
Bug fix for index-out-of-bounds error when attempting to parse the status message string for status code and the delimiter "Status" is not found.
extract_status_code_from_error returns None if the delimiter is not found or an invalid status code is given in the error message
Add error handling messages for status code parsing
Add unit tests to validate changes
@maherthomsi maherthomsi force-pushed the index_out_of_bounds_fix branch from 36ba027 to 608721f Compare September 11, 2025 23:59
@maherthomsi
Copy link
Contributor Author

Commit message changes

@maherthomsi maherthomsi merged commit 903fab1 into bottlerocket-os:develop Sep 12, 2025
2 checks passed
@maherthomsi maherthomsi deleted the index_out_of_bounds_fix branch September 12, 2025 22:17
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.

4 participants