Skip to content

fix: remove dangling placeholder from event source close warning - #3532

Merged
csviri merged 1 commit into
operator-framework:mainfrom
csviri:fix/logging-lost-stack-traces
Aug 3, 2026
Merged

fix: remove dangling placeholder from event source close warning#3532
csviri merged 1 commit into
operator-framework:mainfrom
csviri:fix/logging-lost-stack-traces

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

log.warn("Error closing {} -> {}", eventSource.name(), e) supplies two
placeholders but only one non-throwable argument. SLF4J trims a trailing
Throwable and reports it as the exception, so the stack trace is logged
correctly, but the second placeholder is left unfilled and the message
renders with a literal brace pair:

Error closing myEventSource -> {}

Drops the redundant placeholder and names what is being logged.

Note: the superficially similar calls in InformerManager.stop and
InformerWrapper.start are correct as they stand - they pass more
non-throwable arguments than placeholders, so nothing is left dangling.

Part of #3517

Copilot AI review requested due to automatic review settings July 30, 2026 09:05
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a logging-formatting issue in EventSourceManager where a SLF4J warning message had one placeholder too many, causing the rendered log line to include a literal {} when stopping an event source fails.

Changes:

  • Remove the redundant placeholder from the “error closing” warning.
  • Update the message text to explicitly name what is being closed (“event source”) while preserving exception logging.

@csviri
csviri marked this pull request as ready for review August 1, 2026 19:23
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 1, 2026
@openshift-ci
openshift-ci Bot requested review from metacosm and xstefank August 1, 2026 19:23
`log.warn("Error closing {} -> {}", eventSource.name(), e)` supplies two
placeholders but only one non-throwable argument. SLF4J trims a trailing
`Throwable` and reports it as the exception, so the stack trace is logged
correctly, but the second placeholder is left unfilled and the message
renders with a literal brace pair:

    Error closing myEventSource -> {}

Drops the redundant placeholder and names what is being logged.

Note: the superficially similar calls in `InformerManager.stop` and
`InformerWrapper.start` are correct as they stand - they pass more
non-throwable arguments than placeholders, so nothing is left dangling.
Copilot AI review requested due to automatic review settings August 1, 2026 19:26
@csviri
csviri force-pushed the fix/logging-lost-stack-traces branch from dd44f0f to 8b86b6d Compare August 1, 2026 19:26

@csviri csviri left a comment

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.

LGTM

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@csviri
csviri merged commit 778e42b into operator-framework:main Aug 3, 2026
28 checks passed
csviri added a commit to csviri/java-operator-sdk that referenced this pull request Aug 3, 2026
…rator-framework#3532)

`log.warn("Error closing {} -> {}", eventSource.name(), e)` supplies two
placeholders but only one non-throwable argument. SLF4J trims a trailing
`Throwable` and reports it as the exception, so the stack trace is logged
correctly, but the second placeholder is left unfilled and the message
renders with a literal brace pair:

    Error closing myEventSource -> {}

Drops the redundant placeholder and names what is being logged.

Note: the superficially similar calls in `InformerManager.stop` and
`InformerWrapper.start` are correct as they stand - they pass more
non-throwable arguments than placeholders, so nothing is left dangling.
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