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.

Sign up for GitHub

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

Jump to bottom

Fixes for several string formats, they are based on static analysis. #6306

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Fixes for several string formats, they are based on static analysis. #6306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
441ad6e
Fixes for several string formats, they are based on static analysis.
dk2k Jun 30, 2024
23641f6
Removed several collections which are updated, but seem to be never a…
dk2k Jun 30, 2024
e557c1c
Revert "Removed several collections which are updated, but seem to be…
dk2k Jun 30, 2024
bc06523
fixed placeholders
dk2k Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...tocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/DefaultSamplerCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ protected void computeFromPostBody(HTTPSamplerBase sampler,
HTTPFileArg [] files = {new HTTPFileArg(out.getPath(),"",contentType)};
sampler.setHTTPFiles(files);
} catch (IOException e) {
log.warn("Could not create binary file: {}", e);
log.warn("Could not create binary file", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want the complete stacktrace to be shown here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The last "exception" parameter should not be present in the string format message, so ("Could not create binary file", e); is the right way to go to show the stacktrace

}
} else {
// Just put the whole postbody as the value of a parameter
Expand Down
4 changes: 2 additions & 2 deletions src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ private void initClass() {
javaClass = Class.forName(name, false, Thread.currentThread().getContextClassLoader());
Method method = javaClass.getMethod("teardownTest", JavaSamplerContext.class);
isToBeRegistered = !method.getDeclaringClass().equals(AbstractJavaSamplerClient.class);
log.info("Created class: {}. Uses tearDownTest: ", name, isToBeRegistered);
log.info("Created class: {}. Uses tearDownTest: {}", name, isToBeRegistered);
} catch (Exception e) {
log.error("{}\tException initialising: ", whoAmI(), name, e);
log.error("{}\tException initialising: {} {}", whoAmI(), name, e.getMessage());
Copy link
Collaborator

Choose a reason for hiding this comment

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

The exception can be the last parameter without a placeholder, and it will render as a stacktrace, so .getMessage is not needed here.

Suggested change
log.error("{}\tException initialising: {} {}", whoAmI(), name, e.getMessage());
log.error("{}\tException initialising: {}", whoAmI(), name, e);

}

}
Expand Down
2 changes: 1 addition & 1 deletion src/protocol/tcp/src/main/java/org/apache/jmeter/protocol/tcp/sampler/TCPSampler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ private void closeSocket(String socketKey) {
try {
con.close();
} catch (IOException e) {
log.warn("Error closing socket {}", e); //$NON-NLS-1$
log.warn("Error closing socket", e); //$NON-NLS-1$
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I think it is better to keep the short (string version) of the message here.

}
}
}
Expand Down
Loading