From 7a7bd758a4f9fbdfe97d8100ea93c4d1bc334882 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Tue, 1 Sep 2026 23:52:49 +0200 Subject: [PATCH 1/2] Let a failed request report itself Two places where the code that exists to explain a failure fails instead, and the caller is shown the second failure rather than the first. _do_web_request builds its message from the response body when that body is not the JSON error document the spec asks for. An html 404 page is exactly that, the body is bytes, and str + bytes raised TypeError. The status and the body never reached anyone. LenovoFirmwareConfig raised a bare Exception when python-lxml and python-eficompressor are absent. Confluent has no handler for one, so a missing dependency showed as "Unexpected Error" and hid a message that already said what to install. Neither changes what fails, only what the caller is told. --- confluent_server/aiohmi/ipmi/oem/lenovo/config.py | 8 ++++++-- confluent_server/aiohmi/redfish/oem/generic.py | 9 ++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/confluent_server/aiohmi/ipmi/oem/lenovo/config.py b/confluent_server/aiohmi/ipmi/oem/lenovo/config.py index 7ae16c66..35e72d3b 100644 --- a/confluent_server/aiohmi/ipmi/oem/lenovo/config.py +++ b/confluent_server/aiohmi/ipmi/oem/lenovo/config.py @@ -147,8 +147,12 @@ def _eval_conditional(expression, cfg, setting): class LenovoFirmwareConfig(object): def __init__(self, xc, useipmi=True): if not etree: - raise Exception("python-lxml and python-eficompressor required " - "for this function") + # Not a bare Exception. Confluent has no handler for one, so a + # missing optional dependency showed as "Unexpected Error" and hid + # the message below, which already names what to install. + raise pygexc.UnsupportedFunctionality( + "python-lxml and python-eficompressor required " + "for this function") if useipmi: self.connection = xc.ipmicmd else: diff --git a/confluent_server/aiohmi/redfish/oem/generic.py b/confluent_server/aiohmi/redfish/oem/generic.py index b44bc253..ba739fb4 100644 --- a/confluent_server/aiohmi/redfish/oem/generic.py +++ b/confluent_server/aiohmi/redfish/oem/generic.py @@ -1794,7 +1794,14 @@ class OEMHandler(object): errmsg = ','.join(errmsg) raise exc.RedfishError(errmsg) except (ValueError, KeyError): - raise exc.PyghmiException(str(url) + ":" + res[0]) + # Reached by an html 404 page, or anything else that is not + # the JSON error document. res[0] is bytes there, so building + # the message crashed and the caller saw that TypeError + # instead of the status and the body. + body = res[0] + if isinstance(body, bytes): + body = body.decode('utf-8', errors='replace') + raise exc.PyghmiException('{0}:{1}'.format(url, body)) if payload is None and method is None: self._urlcache[url] = { 'contents': res[0], From 9012888cc0bcb06074d84da3d5198dce090719d8 Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Tue, 1 Sep 2026 23:52:49 +0200 Subject: [PATCH 2/2] Report a refused XCC web login instead of returning None get_webclient falls off its end when /api/login answers anything but 200, so it returned None. wc() passes that back, and thirty of the thirty-four call sites use it unchecked, so a refused login arrived as "'NoneType' object has no attribute 'grab_json_response'" from wherever it landed. Raised where the failure is known, and with the status: 404 is firmware with no web api, or a Redfish-only capture of one, while 401 is credentials it will not take. Neither was distinguishable before. The other four call sites are the inventory reads, and they always did check. They answer partially when the web interface is out of reach, which is why nodeinventory still says something useful. They ask through wc_if_available now, so that tolerance is stated rather than resting on a None. --- .../aiohmi/redfish/oem/lenovo/xcc.py | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py b/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py index d9a9d173..a2d01fc6 100644 --- a/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py +++ b/confluent_server/aiohmi/redfish/oem/lenovo/xcc.py @@ -1150,6 +1150,18 @@ class OEMHandler(generic.OEMHandler): self.weblogging = False return self._wc + async def wc_if_available(self): + """The web client, or None where carrying on without it is meant. + + Four inventory reads answer partially when the web interface is out of + reach, and used to say so by checking wc() for None. wc() refuses now, + so they ask here instead. + """ + try: + return await self.wc() + except pygexc.UnsupportedFunctionality: + return None + async def get_webclient(self, login=True): wc = self.webclient.dupe() wc.vintage = util._monotonic_time() @@ -1179,6 +1191,12 @@ class OEMHandler(generic.OEMHandler): wc.set_header('X-XSRF-TOKEN', cookie.value) break return wc + # Falling off the end returned None, which wc() handed to thirty + # call sites that use it unchecked. The status separates firmware + # with no web api, answering 404, from credentials it will not take. + raise pygexc.UnsupportedFunctionality( + 'the XCC web interface, which this operation needs beside ' + 'Redfish, did not accept a login (HTTP {0})'.format(status)) async def grab_redfish_response_with_status(self, url, body=None, method=None): wc = self.webclient @@ -1818,7 +1836,7 @@ class OEMHandler(generic.OEMHandler): if self.updating: raise pygexc.TemporaryError( 'Cannot read extended inventory during firmware update') - wc = await self.wc() + wc = await self.wc_if_available() if wc: adapterdata = await wc.grab_json_response(self.ADP_URL) if adapterdata: @@ -1897,7 +1915,7 @@ class OEMHandler(generic.OEMHandler): # mode 0 is firmware, 1 is hardware storagedata = self.get_cached_data('lenovo_cached_storage') if not storagedata: - wc = await self.wc() + wc = await self.wc_if_available() if wc: storagedata = await wc.grab_json_response( '/api/function/raid_alldevices?params=storage_GetAllDisks') @@ -1926,7 +1944,7 @@ class OEMHandler(generic.OEMHandler): async def _get_cpu_inventory(self): procdata = self.get_cached_data('lenovo_cached_proc') if not procdata: - wc = await self.wc() + wc = await self.wc_if_available() if wc: procdata = await wc.grab_json_response( '/api/dataset/imm_processors') @@ -1944,7 +1962,7 @@ class OEMHandler(generic.OEMHandler): async def _get_mem_inventory(self): memdata = self.get_cached_data('lenovo_cached_memory') if not memdata: - wc = await self.wc() + wc = await self.wc_if_available() if wc: memdata = await wc.grab_json_response( '/api/dataset/imm_memory')