The library depends on slf4j-api 2.0.17 (the only runtime dependency in pom.xml) and logs from 25 classes in src/main (38 call sites: 19 error, 11 warn, 2 info, 6 debug). Nothing in src/test logs. There is no Log4J dependency, only SLF4J; whoever embeds the library decides the binding, and without one every message is silently dropped (SLF4J(W): No SLF4J providers were found shows up in every build).
A library should not decide how problems are reported: callers get the information through exceptions and return values, and the dependency goes away.
Inventory of the 38 call sites, by what should replace them
1. Logged exceptions in catch blocks that then continue (19 sites) → propagate or return a result
| Location |
Today |
Replacement |
IpmiAsyncConnector ×5 (:229, :273, :330, :385, :441) |
warn on each retry, rethrows after the last |
drop the log; the final exception already carries the cause (consider addSuppressed for the earlier attempts) |
IpmiConnector:447 |
warn "Receiving message failed, retrying" |
same |
GetFrusRunner ×3 (:204, :256, :272) |
warn when a FRU read / chunk / decode fails (added in #114 to replace empty catches) |
return the failure with the result: a FRU that could not be read should be reported to the caller, not silently truncated (#102) |
AbstractIpmiRunner:222 |
debug on closeSession failure in close() |
throw, or keep ignoring: close() is best effort, the javadoc should say so |
SessionManager:93, Connection:330, :650, :681 |
error then continue |
propagate; Connection:650 is the state machine's ErrorAction, which already holds the exception |
UdpMessenger:143, :146 |
error in the receive loop |
the receive thread has no caller: surface the error through the listener (UdpListener) or fail the pending requests (ties into #79) |
InboundSolMessageListener:129, :148, :162 |
error when an SOL ACK/NACK cannot be sent |
same pattern: the SOL listener thread needs an error callback |
SerialOverLan:736 |
error "Error while sending message" |
throw |
BoardInfo:90, PropertiesManager:65 |
error(e.getMessage(), e) |
throw |
2. Unknown enum values (7 sites) → return a value
ChassisType:171, FruMultiRecordType:91, ManagementAccessRecordType:70, DeviceType:175, EntityId:354, SensorType:234, SensorUnit:535 all log "Invalid value: " + value from parseInt and then throw or return a default. Per the IPMI specifics in AGENTS.md (OEM record types 0xC0-0xFF are legitimate), these should return an Unknown/Oem constant carrying the raw code, no log, no exception. Overlaps with #112's SDR walk work.
3. Decoder warnings (2 sites) → carry them in the result
Protocolv20Decoder:142 "Integrity check failed" and DeactivatePayload:127 (completion code message). An integrity failure on a received packet must reject the packet (exception) rather than warn and continue. The AGENTS.md rule "skip, log and continue" for unmodelled SDR/FRU records needs a non-logging form: the decoded result (e.g. the Fru/sensor list) should expose the records that were skipped and why, so MetricsHub can report them.
4. Trace/debug noise (10 sites) → delete
IpmiConnector:421, IpmiMessageHandler:63, MessageHandler:161, SolMessageHandler:72, :75, MessageQueue:358, PropertiesManager:70 (logs every property read at info), and the three debug sites above. Delete; the unit tests and the harness approach cover what these were for.
Scope
Acceptance
Related: #79 (timeouts/cancellation on the receive threads), #102 (truncated FRUs), #112 (SDR walk), #114 (the warn calls it added are to be replaced here).
The library depends on
slf4j-api2.0.17 (the only runtime dependency inpom.xml) and logs from 25 classes insrc/main(38 call sites: 19error, 11warn, 2info, 6debug). Nothing insrc/testlogs. There is no Log4J dependency, only SLF4J; whoever embeds the library decides the binding, and without one every message is silently dropped (SLF4J(W): No SLF4J providers were foundshows up in every build).A library should not decide how problems are reported: callers get the information through exceptions and return values, and the dependency goes away.
Inventory of the 38 call sites, by what should replace them
1. Logged exceptions in
catchblocks that then continue (19 sites) → propagate or return a resultIpmiAsyncConnector×5 (:229,:273,:330,:385,:441)warnon each retry, rethrows after the lastaddSuppressedfor the earlier attempts)IpmiConnector:447warn"Receiving message failed, retrying"GetFrusRunner×3 (:204,:256,:272)warnwhen a FRU read / chunk / decode fails (added in #114 to replace empty catches)AbstractIpmiRunner:222debugoncloseSessionfailure inclose()close()is best effort, the javadoc should say soSessionManager:93,Connection:330,:650,:681errorthen continueConnection:650is the state machine'sErrorAction, which already holds the exceptionUdpMessenger:143,:146errorin the receive loopUdpListener) or fail the pending requests (ties into #79)InboundSolMessageListener:129,:148,:162errorwhen an SOL ACK/NACK cannot be sentSerialOverLan:736error"Error while sending message"BoardInfo:90,PropertiesManager:65error(e.getMessage(), e)2. Unknown enum values (7 sites) → return a value
ChassisType:171,FruMultiRecordType:91,ManagementAccessRecordType:70,DeviceType:175,EntityId:354,SensorType:234,SensorUnit:535all log"Invalid value: " + valuefromparseIntand then throw or return a default. Per the IPMI specifics in AGENTS.md (OEM record types 0xC0-0xFF are legitimate), these should return anUnknown/Oemconstant carrying the raw code, no log, no exception. Overlaps with #112's SDR walk work.3. Decoder warnings (2 sites) → carry them in the result
Protocolv20Decoder:142"Integrity check failed" andDeactivatePayload:127(completion code message). An integrity failure on a received packet must reject the packet (exception) rather than warn and continue. The AGENTS.md rule "skip, log and continue" for unmodelled SDR/FRU records needs a non-logging form: the decoded result (e.g. theFru/sensor list) should expose the records that were skipped and why, so MetricsHub can report them.4. Trace/debug noise (10 sites) → delete
IpmiConnector:421,IpmiMessageHandler:63,MessageHandler:161,SolMessageHandler:72,:75,MessageQueue:358,PropertiesManager:70(logs every property read atinfo), and the threedebugsites above. Delete; the unit tests and the harness approach cover what these were for.Scope
slf4j-apifrompom.xmland everyorg.slf4jimport;mvn dependency:treeshows no runtime dependencies afterwards.Acceptance
grep -r org.slf4j srcis empty and the build no longer prints the SLF4J no-provider warning.EmptyCatchBlockis gated by PMD: fix the 117 violations reported with the project ruleset (empty catch blocks, Thread.run(), always-true if, unary operators, qualified names, parentheses) #114, so a bare catch cannot come back).mvn verifypasses with 0 Checkstyle and 0 PMD violations.Related: #79 (timeouts/cancellation on the receive threads), #102 (truncated FRUs), #112 (SDR walk), #114 (the warn calls it added are to be replaced here).