[ALL] Use of assert checks in code
I'm not convinced that it ever makes sense to use assert checks in our code. In the case of test code, it seems to me that a JUnit assertion makes more sense, as it is unconditionally checked. In main code, does it make sense to use a check that is not guaranteed to run? I would expect to see an IllegalStateException instead if the check fails. However, if an assert IS needed, in most cases it needs a message to ensure any failures contain sufficient information to determine the cause of the failure. For example: assert length >= 10; // bad: what is the value of length? assert length >= 10 : "Length: " + length; // good: show unexpected value assert isEmpty; // does not need a message Sebb - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [ALL] Use of assert checks in code
Java assertions only make sense in specific cases. Never in test code, use JUnit for that (or TestNG). It might make sense in low level libraries to enable some semantic checking which can be enabled on the CLI. This might be more convenient instead of "if debug is on AND some condition then throw a runtime exception". The above would be orthogonal to using a logging framework. In general though, I don't use Java's assert keyword. Gary On Thu, Jul 25, 2024, 12:24 PM sebb wrote: > I'm not convinced that it ever makes sense to use assert checks in our > code. > > In the case of test code, it seems to me that a JUnit assertion makes > more sense, as it is unconditionally checked. > > In main code, does it make sense to use a check that is not guaranteed to > run? > I would expect to see an IllegalStateException instead if the check fails. > > However, if an assert IS needed, in most cases it needs a message to > ensure any failures contain sufficient information to determine the > cause of the failure. > > For example: > > assert length >= 10; // bad: what is the value of length? > > assert length >= 10 : "Length: " + length; // good: show unexpected value > > assert isEmpty; // does not need a message > > Sebb > > - > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
Re: [ALL] Use of assert checks in code
On Thu, 25 Jul 2024 at 17:46, Gary Gregory wrote: > > Java assertions only make sense in specific cases. > > Never in test code, use JUnit for that (or TestNG). > > It might make sense in low level libraries to enable some semantic checking > which can be enabled on the CLI. This might be more convenient instead of > "if debug is on AND some condition then throw a runtime exception". > > The above would be orthogonal to using a logging framework. > > In general though, I don't use Java's assert keyword. What about the following? https://github.com/apache/commons-io/blob/c155293776f49eef93547aca9cf181c8a0591cad/src/main/java/org/apache/commons/io/input/ReadAheadInputStream.java#L488 https://github.com/apache/commons-io/blob/c155293776f49eef93547aca9cf181c8a0591cad/src/main/java/org/apache/commons/io/input/ReadAheadInputStream.java#L498 https://github.com/apache/commons-io/blob/c155293776f49eef93547aca9cf181c8a0591cad/src/main/java/org/apache/commons/io/input/ThrottledInputStream.java#L130 https://github.com/apache/commons-io/blob/c155293776f49eef93547aca9cf181c8a0591cad/src/main/java/org/apache/commons/io/input/ThrottledInputStream.java#L150 > Gary > > On Thu, Jul 25, 2024, 12:24 PM sebb wrote: > > > I'm not convinced that it ever makes sense to use assert checks in our > > code. > > > > In the case of test code, it seems to me that a JUnit assertion makes > > more sense, as it is unconditionally checked. > > > > In main code, does it make sense to use a check that is not guaranteed to > > run? > > I would expect to see an IllegalStateException instead if the check fails. > > > > However, if an assert IS needed, in most cases it needs a message to > > ensure any failures contain sufficient information to determine the > > cause of the failure. > > > > For example: > > > > assert length >= 10; // bad: what is the value of length? > > > > assert length >= 10 : "Length: " + length; // good: show unexpected value > > > > assert isEmpty; // does not need a message > > > > Sebb > > > > - > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[EMAIL] Serious network crash bug for commons email 2 library
Hi everyone,
I am a senior software engineer working on a TomEE project and as part of our
recent upgrade to Jakarta 10 and Java 17, we also upgraded the Apache Commons
email library to version 2 which you just released.
We encountered a very critical bug which only occurs with certain hardware and
drivers, but it has very serious impacts on the host OS and would be
catastrophic if it were to happen on production systems. I suggest you look
into this as a matter of urgency because I would not want anyone else to
encounter this bug as it is very bad, but also very obscure and could easily be
missed during testing.
PROBLEM DESCRIPTION:
On certain physical hardware, with a specific OS network driver installed, if
the Apache commons email2 library is used and an email is sent, the email fails
to send, and the OS network driver crashes and the computer loses all network
access. The network access can only be restored by restarting the computer,
sometimes multiple times.
HARDWARE & OS:
Computer: Optiplex 7000
Windows 11
Ethernet driver: Intel 12.19.2.45
SOFTWARE:
Java 17
Jakarta 10
TomEE 9.1.2
These dependencies caused the issue:
commons-mail = { module = "org.apache.commons:commons-email2-jakarta", version
= "2.0.0-M1" }
commons-mail-core = { module = "org.apache.commons:commons-email2-core",
version = "2.0.0-M1" }
We tried both the snapshot version from a few months ago, and the final release
version from a few weeks ago.
Snapshot repo: https://repository.apache.org/content/repositories/snapshots/
Release repo:
https://mvnrepository.com/artifact/org.apache.commons/commons-email2-jakarta/2.0.0-M1
In the Java code:
import org.apache.commons.mail2.core.EmailException;
import org.apache.commons.mail2.jakarta.HtmlEmail;
STEPS TO REPRODUCE:
1. Make sure you are testing on a computer with the same physical
hardware/network card
2. Install the exact same version of the Intel ethernet driver. This issue
does not occur for older/newer versions of the Intel driver
3. Run TomEE9 with the apache-commons-email2 library in the libs folder of a
WAR deployment.
4. Send an email.
5. The email will never get sent across the network.
6. The computer will lose all network access.
7. Network access will only be restored after restarting the computer.
DEBUG NOTES:
While debugging the Java stack trace, we found that if the commons-email2
libraries are used, somehow one of the Java classes in geronimo-mail is called
which is part of the TomEE/libs and this crashes the computer network. So it
might be an issue with commons-email2 communicating with certain 3rd-party
libraries in combination with certain hardware/drivers.
STEPS TO FIX:
If we add an excludes for the Geronimo library, and swap apache-commons-email
with a different email implementation library, it seems fine.
Please also note if I update my network driver to the latest Intel version,
2.0.0-M1 doesn't crash my network. It also doesn't crash with older driver
versions, only version 12.19.2.45. This is not an issue for Linux systems.
Please let me know if you need any more information about how to reproduce the
issue.
Regards,
Adrian
