Skip to content

Conversation

@onelapahead
Copy link
Contributor

@onelapahead onelapahead commented Dec 12, 2025

Following up from #201

Its a bit awkward - we added a private field to HandlerFactory. But HandlerFactory doesn't have a proper New func or Init to guarantee that internal log level field is set. If the log level field is not set and defaults to 0, then logrus panics on the first log message.

For most clients, they let APIServer construct and setup a HandlerFactory, so they are unaffected. But if anyone was initializing their own HandlerFactory and doesn't then call SetAPIEntryLoggingLevel, it would panic.

We don't want to check for 0 on every API request, as that would be inefficient, so we check when we construct APIWrapper's who would depend on the log level to not be nil in order to fix this.

In a 2.0 of ff-common, we should likely make all fields of HandlerFactory private, have a New func, and use the functional options or builder pattern to provide a variadic way of setting all the possible fields (which is what a struct with public fields and no constructor feels like, but has potential for bugs like this one).

Signed-off-by: hfuss <hayden.fuss@kaleido.io>
@onelapahead onelapahead requested a review from a team as a code owner December 12, 2025 13:42
Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

Thanks @onelapahead !

@EnriqueL8 EnriqueL8 merged commit 300d3ce into hyperledger:main Dec 12, 2025
5 of 6 checks passed
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.

2 participants