Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Task] Update spotbugs version #145

Closed
andrewazores opened this issue Jun 9, 2022 · 2 comments · Fixed by #153
Closed

[Task] Update spotbugs version #145

andrewazores opened this issue Jun 9, 2022 · 2 comments · Fixed by #153
Assignees
Labels
chore Refactor, rename, cleanup, etc. dependencies Pull requests that update a dependency file good first issue Good for newcomers

Comments

@andrewazores
Copy link
Member

Currently using spotbugs version 4.2.3, but 4.7.0 is out. This version should be bumped and any new bugs found fixed accordingly.

@andrewazores andrewazores added chore Refactor, rename, cleanup, etc. dependencies Pull requests that update a dependency file good first issue Good for newcomers labels Jun 9, 2022
@maxcao13 maxcao13 self-assigned this Aug 9, 2022
@maxcao13
Copy link
Member

maxcao13 commented Aug 9, 2022

Seems like the update is causing a lot of new THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION / RUNTIME spotbugs errors https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#throws-method-lists-exception-in-its-throws-clause-throws-method-throws-clause-basic-exception. Around 35-40 in -core. Other repos in other orgs have issues with false positives with Junit 5 AssertThrowables and other issues, but I don't think we have that here.
e.g.

INFO] BugInstance size is 43
[INFO] Error size is 0
[INFO] Total bugs: 43
[ERROR] Medium: io.cryostat.core.agent.AgentJMXHelper.getMBeanServerConnection() may expose internal representation by returning AgentJMXHelper.mbsc [io.cryostat.core.agent.AgentJMXHelper] At AgentJMXHelper.java:[line 73] EI_EXPOSE_REP
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.AgentJMXHelper] At AgentJMXHelper.java:[lines 118-125] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.AgentJMXHelper] At AgentJMXHelper.java:[lines 78-81] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.AgentJMXHelper] At AgentJMXHelper.java:[lines 103-112] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.AgentJMXHelper] At AgentJMXHelper.java:[lines 87-97] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: new io.cryostat.core.agent.LocalProbeTemplateService(FileSystem, Environment) may expose internal representation by storing an externally mutable object into LocalProbeTemplateService.fs [io.cryostat.core.agent.LocalProbeTemplateService] At LocalProbeTemplateService.java:[line 64] EI_EXPOSE_REP2
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.LocalProbeTemplateService] At LocalProbeTemplateService.java:[lines 89-105] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.LocalProbeTemplateService] At LocalProbeTemplateService.java:[lines 108-112] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.LocalProbeTemplateService] At LocalProbeTemplateService.java:[lines 115-118] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method intentionally throws RuntimeException. [io.cryostat.core.agent.ProbeTemplate] At ProbeTemplate.java:[lines 174-196] THROWS_METHOD_THROWS_RUNTIMEEXCEPTION
[ERROR] Medium: Method intentionally throws RuntimeException. [io.cryostat.core.agent.ProbeTemplate] At ProbeTemplate.java:[lines 95-147] THROWS_METHOD_THROWS_RUNTIMEEXCEPTION
[ERROR] Medium: Method intentionally throws RuntimeException. [io.cryostat.core.agent.ProbeTemplate] At ProbeTemplate.java:[lines 150-170] THROWS_METHOD_THROWS_RUNTIMEEXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.agent.ProbeTemplateService] In ProbeTemplateService.java THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnection] At JFRConnection.java:[lines 96-97] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnection] At JFRConnection.java:[lines 85-92] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnection] At JFRConnection.java:[lines 206-215] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnection] At JFRConnection.java:[lines 162-182] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnection] At JFRConnection.java:[lines 104-114] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method intentionally throws RuntimeException. [io.cryostat.core.net.JFRConnection] At JFRConnection.java:[line 220] THROWS_METHOD_THROWS_RUNTIMEEXCEPTION
[ERROR] Medium: new io.cryostat.core.net.JFRConnectionToolkit(ClientWriter, FileSystem, Environment) may expose internal representation by storing an externally mutable object into JFRConnectionToolkit.fs [io.cryostat.core.net.JFRConnectionToolkit] At JFRConnectionToolkit.java:[line 66] EI_EXPOSE_REP2
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnectionToolkit] At JFRConnectionToolkit.java:[line 71] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnectionToolkit] At JFRConnectionToolkit.java:[line 75] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.net.JFRConnectionToolkit] At JFRConnectionToolkit.java:[lines 80-88] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: new io.cryostat.core.reports.InterruptibleReportGenerator(Logger, Set, ExecutorService) may expose internal representation by storing an externally mutable object into InterruptibleReportGenerator.transformers [io.cryostat.core.reports.InterruptibleReportGenerator] At InterruptibleReportGenerator.java:[line 105] EI_EXPOSE_REP2
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.reports.InterruptibleReportGenerator] At InterruptibleReportGenerator.java:[lines 340-352] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.reports.InterruptibleReportGenerator] At InterruptibleReportGenerator.java:[lines 174-191] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.reports.InterruptibleReportGenerator] At InterruptibleReportGenerator.java:[lines 123-154] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.AbstractTemplateService] In AbstractTemplateService.java THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.AbstractTemplateService] At AbstractTemplateService.java:[line 51] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: new io.cryostat.core.templates.LocalStorageTemplateService(FileSystem, Environment) may expose internal representation by storing an externally mutable object into LocalStorageTemplateService.fs [io.cryostat.core.templates.LocalStorageTemplateService] At LocalStorageTemplateService.java:[line 81] EI_EXPOSE_REP2
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.LocalStorageTemplateService] At LocalStorageTemplateService.java:[line 72] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.MergedTemplateService] At MergedTemplateService.java:[lines 88-94] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.MergedTemplateService] At MergedTemplateService.java:[lines 67-70] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.MergedTemplateService] At MergedTemplateService.java:[lines 75-81] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.RemoteTemplateService] At RemoteTemplateService.java:[lines 106-110] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.RemoteTemplateService] At RemoteTemplateService.java:[line 132] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.RemoteTemplateService] At RemoteTemplateService.java:[line 62] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.RemoteTemplateService] At RemoteTemplateService.java:[lines 77-80] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.TemplateService] At TemplateService.java:[line 59] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.TemplateService] In TemplateService.java THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.TemplateService] In TemplateService.java THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.TemplateService] At TemplateService.java:[line 53] THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION
[ERROR] Medium: Method lists Exception in its throws clause. [io.cryostat.core.templates.TemplateService] In TemplateService.java THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION

For example this method

public void defineEventProbes(String xmlDescription) throws Exception {

throws a general Exception exception. To 'solve' the bug, it could be replaced with

throws InstanceNotFoundException, MalformedObjectNameException, MBeanException, ReflectionException, IOException

And I'm not sure if that's what we want. I could also look into replacing the exception with a new class of exception, or just suppress them all together, @andrewazores WDYT?

@andrewazores
Copy link
Member Author

For that specific example I think creating something like a ProbeDefinitionException class would make sense, and then just wrap the implementation with a try/catch that catches all those other types of exception and wraps them into a ProbeDefinitionException. In other places just replacing the throws clause on the method with an expanded list of the checked exception types is probably more suitable, it'll just have to be decided on a case-by-case basis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. dependencies Pull requests that update a dependency file good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants