Skip to content

Add robust log level handling in JVSGLoggerLogManager#3287

Open
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
HeikoKlare:jsvglogger-level-config
Open

Add robust log level handling in JVSGLoggerLogManager#3287
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
HeikoKlare:jsvglogger-level-config

Conversation

@HeikoKlare
Copy link
Copy Markdown
Contributor

The JVSGLoggerLogManager allows to customize the log level with a system property. When an invalid log level is passed, the call currently throws an exception and makes the whole rasterizer initialization fail, leading to the non-availability of an SVG rasterizer throughout the whole application lifecycle.

This change makes the system property processing more robust: when an invalid level is used, an error is printed and the implementation falls back to using "ERROR" as default log level. In addition, capitalization is ignored by always making the passed value upper case. As a result of this change, it not be possible to make the rasterizer implementation fail with an invalid system property anymore.

Resolves #3283 (comment)

The JVSGLoggerLogManager allows to customize the log level with a system
property. When an invalid log level is passed, the call currently throws
an exception and makes the whole rasterizer initialization fail, leading
to the non-availability of an SVG rasterizer throughout the whole
application lifecycle.

This change makes the system property processing more robust: when an
invalid level is used, an error is printed and the implementation falls
back to using "ERROR" as default log level. In addition, capitalization
is ignored by always making the passed value upper case. As a result of
this change, it not be possible to make the rasterizer implementation
fail with an invalid system property anymore.
@HeikoKlare HeikoKlare requested a review from merks May 6, 2026 19:14
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Test Results

  182 files  ±0    182 suites  ±0   25m 7s ⏱️ -28s
4 723 tests ±0  4 700 ✅ ±0   23 💤 ±0  0 ❌ ±0 
6 812 runs  ±0  6 649 ✅ ±0  163 💤 ±0  0 ❌ ±0 

Results for commit dee73cc. ± Comparison against base commit c1fd7a0.

Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

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

Looks better. Just a tiny name consistency issue.


import com.github.weisj.jsvg.logging.LogManager;
import com.github.weisj.jsvg.logging.Logger;
import com.github.weisj.jsvg.logging.Logger.Level;
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.

Maybe use the Logger.Level like the other parts to avoid this import? Or make the rest consistent?

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