Skip to content

Fix check bom http error reporting #237

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.

Sign up for GitHub

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

Open
gernot-h wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix check bom http error reporting #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
93f0600
test(check_bom): fix and re-enable tests for error cases
gernot-h Jul 29, 2026
543bd2d
fix(bom check): inform user about HTTP errors when getting release
gernot-h Jul 29, 2026
f08d637
fix(project create): inform user about HTTP errors when updating project
gernot-h Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions capycli/bom/check_bom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def _find_by_id(self, component: Component) -> Optional[Dict[str, Any]]:
print(
" " + component.name + ", " + version +
", " + sw360id)
if swex.response:
if swex.response is not None:
print(" Status Code: " + str(swex.response.status_code))
if swex.message:
print(" Message: " + swex.message)
Expand Down Expand Up @@ -105,7 +105,7 @@ def _find_by_name(self, component: Component) -> Optional[Dict[str, Any]]:
print(Fore.LIGHTRED_EX + " Error retrieving release data: ")
print(
" " + component.name + ", " + version)
if swex.response:
if swex.response is not None:
print(" Status Code: " + str(swex.response.status_code))
if swex.message:
print(" Message: " + swex.message)
Expand Down
9 changes: 1 addition & 8 deletions capycli/project/create_project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,7 @@ def update_project(self, project_id: str, project: Optional[Dict[str, Any]],
if swex.response.status_code == requests.codes["forbidden"]:
print_red(" You are not authorized - do you have a valid write token?")
sys.exit(ResultCode.RESULT_AUTH_ERROR)
if swex.response:
print_red(" " + str(swex.response.status_code) + ": " + swex.response.text)
sys.exit(ResultCode.RESULT_ERROR_ACCESSING_SW360)
if swex.details:

@tngraf tngraf Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why has this code been removed?
response.text is one possible problem description.
SW360s real description is in swex.details["error"] and swex.details["message"]

@gernot-h gernot-h Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is the current control flow from https://github.com/sw360/capycli/blob/v2.11.1/capycli/project/create_project.py#L139:

            if swex.response is None:  # 1
                ...
                sys.exit(...)
            if swex.response.status_code == requests.codes["unauthorized"]:  # 2
                ...
                sys.exit(...)
            if swex.response.status_code == requests.codes["forbidden"]:  # 3
                ...
                sys.exit(...)
            if swex.response:  # 4
                ...
                sys.exit(...)
            if swex.details:  # 5
                ...
                sys.exit(...)

            print_red("  Unknown error ...)  # 6
            sys.exit(...)

Now as in the other error described by Raul, # 4 will never be triggered as .response will evaluate to False for any HTTP error. So we would need to replace # 4 by if swex.response is not None which is what I did first. But then# 4 is the exact opposite of # 1, so mypy shouted at me that neither # 5 nor # 6 are reachable.

Or in other words: # 1 is already the "general error" part which is called when we have no valid .response, so we don't need # 6.

And looking into https://github.com/sw360/sw360python/blob/master/sw360/sw360error.py, swex.details is only populated when we have a valid .response, so # 4 will already cover that case, so we also don't need # 5.

Or do I overlook something?

@gernot-h gernot-h Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The patch is also somewhat confusing, but what I actually did was removing # 5 and # 6 and making the code from # 4 the fall-back if other if's don't mach.

print_red(" " + swex.details.get("error", "") + ": " + swex.details.get("message", ""))
sys.exit(ResultCode.RESULT_ERROR_ACCESSING_SW360)

print_red(" Unknown error updating project: " + repr(swex))
print_red(" " + str(swex.response.status_code) + ": " + swex.response.text)
sys.exit(ResultCode.RESULT_ERROR_ACCESSING_SW360)

def update_project_version(self, project_id: str, project: Dict[str, Any], new_version: str) -> None:
Expand Down
78 changes: 50 additions & 28 deletions tests/test_check_bom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ def test_simple_bom_without_id(self) -> None:
self.assertTrue("wheel, 0.38.4" in out)

@responses.activate
def xx_test_simple_bom_with_errors(self) -> None:
def test_simple_bom_with_errors(self) -> None:
sut = CheckBom()

# create argparse command line argument object
Expand Down Expand Up @@ -361,7 +361,7 @@ def xx_test_simple_bom_with_errors(self) -> None:
responses.GET,
url=self.MYURL + "resource/api/releases/05c30bf89a512463260b57e84d99b38f",
body='{"name": "python", "version": "3.8"}',
status=500, # internal server error
status=403, # forbidden (don't use 500 as this leads to multiple retries by urllib3)
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
)
Expand All @@ -370,7 +370,17 @@ def xx_test_simple_bom_with_errors(self) -> None:
responses.add(
responses.GET,
url=self.MYURL + "resource/api/releases/fa0d21eb17574ba9ae17e5c9b432558e",
body='{"name": "tomli", "version": "2.0.1"}',
body='''
{
"name": "tomli",
"version": "2.0.1",
"_links" : {
"self" : {
"href" : "https://my.server.com/resource/api/releases/fa0d21eb17574ba9ae17e5c9b432558e"
}
}
}
''',
status=200,
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
Expand All @@ -380,7 +390,17 @@ def xx_test_simple_bom_with_errors(self) -> None:
responses.add(
responses.GET,
url=self.MYURL + "resource/api/releases/e0995819173d4ac8b1a4da3548935976",
body='{"name": "wheel", "version": "0.38.4"}',
body='''
{
"name": "wheel",
"version": "0.38.4",
"_links" : {
"self" : {
"href" : "https://my.server.com/resource/api/releases/e0995819173d4ac8b1a4da3548935976"
}
}
}
''',
status=200,
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
Expand All @@ -391,9 +411,10 @@ def xx_test_simple_bom_with_errors(self) -> None:
self.assertTrue("python, 3.8" in out)
self.assertTrue("tomli, 2.0.1" in out)
self.assertTrue("wheel, 0.38.4" in out)
self.assertTrue("Status Code: 403" in out)

@responses.activate
def xx_test_simple_bom_without_id_with_errors(self) -> None:
def test_simple_bom_without_id_with_errors(self) -> None:
sut = CheckBom()

# create argparse command line argument object
Expand All @@ -419,7 +440,17 @@ def xx_test_simple_bom_without_id_with_errors(self) -> None:
responses.add(
responses.GET,
url=self.MYURL + "resource/api/releases/9a2373710bd44769a2560dd31280901d",
body='{"name": "colorama", "version": "0.4.6"}',
body='''
{
"name": "colorama",
"version": "0.4.6",
"_links" : {
"self" : {
"href" : "https://my.server.com/resource/api/releases/9a2373710bd44769a2560dd31280901d"
}
}
}
''',
status=200,
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
Expand All @@ -429,7 +460,17 @@ def xx_test_simple_bom_without_id_with_errors(self) -> None:
responses.add(
responses.GET,
url=self.MYURL + "resource/api/releases/05c30bf89a512463260b57e84d99b38f",
body='{"name": "python", "version": "3.8"}',
body='''
{
"name": "python",
"version": "3.8",
"_links" : {
"self" : {
"href" : "https://my.server.com/resource/api/releases/05c30bf89a512463260b57e84d99b38f"
}
}
}
''',
status=200,
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
Expand All @@ -455,16 +496,6 @@ def xx_test_simple_bom_without_id_with_errors(self) -> None:
adding_headers={"Authorization": "Token " + self.MYTOKEN},
)

# for tomli (2)
responses.add(
responses.GET,
url=self.MYURL + "resource/api/releases/fa0d21eb17574ba9ae17e5c9b432558e",
body='{"name": "tomli", "version": "2.0.1"}',
status=200,
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
)

# for wheel (1)
responses.add(
responses.GET,
Expand All @@ -481,17 +512,7 @@ def xx_test_simple_bom_without_id_with_errors(self) -> None:
}
} ]
}}''',
status=500,
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
)

# for wheel (2)
responses.add(
responses.GET,
url=self.MYURL + "resource/api/releases/e0995819173d4ac8b1a4da3548935976",
body='{"name": "wheel", "version": "0.38.4"}',
status=200,
status=403, # forbidden (don't use 500 as this leads to multiple retries by urllib3)
content_type="application/json",
adding_headers={"Authorization": "Token " + self.MYTOKEN},
)
Expand All @@ -501,6 +522,7 @@ def xx_test_simple_bom_without_id_with_errors(self) -> None:
self.assertTrue("python, 3.8" in out)
self.assertTrue("tomli, 2.0.1" in out)
self.assertTrue("wheel, 0.38.4" in out)
self.assertIn("Status Code: 403", out)

@responses.activate
def xxx_test_simple_bom_show_all(self) -> None:
Expand Down
12 changes: 7 additions & 5 deletions tests/test_update_project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from cyclonedx.model.bom import Bom
from pytest import fixture, raises
from requests import Response
from sw360 import SW360Error

from capycli.main.result_codes import ResultCode
Expand Down Expand Up @@ -51,12 +52,13 @@ class DummyResp:


@fixture
def dummy_response() -> Callable[[int, str], Callable[[int, str], MagicMock]]:
"""Fixture to create a dummy response object."""
def _dummy_response(status_code: int, text: str) -> Callable[[int, str], MagicMock]:
result = MagicMock()
def dummy_response() -> Callable[[int, str], Response]:
"""Fixture to create a dummy response object with real requests.Response
so that bool(response) correctly returns False for non-2xx status codes."""
def _dummy_response(status_code: int, text: str) -> Response:
result = Response()
result.status_code = status_code
result.text = text
result._content = text.encode("utf-8")
return result
return _dummy_response

Expand Down
Loading