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

[java] ensure proper error message gets logged #12853

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Conversation

titusfortner
Copy link
Member

Java Throwable class (at least for Java 11) does not set the detailMessage in the initCause method the same way it does in the way it does in the constructor:

    public Throwable(Throwable cause) {
        fillInStackTrace();
        detailMessage = (cause==null ? null : cause.toString());
        this.cause = cause;
    }

vs

    public synchronized Throwable initCause(Throwable cause) {
        if (this.cause != this)
            throw new IllegalStateException("Can't overwrite cause with " +
                                            Objects.toString(cause, "a null"), this);
        if (cause == this)
            throw new IllegalArgumentException("Self-causation not permitted", this);
        this.cause = cause;
        return this;
    }

@pujagani
Copy link
Contributor

pujagani commented Oct 5, 2023

Good observation. This is a good start. I think similar changes might be required in other places as well. Example:

,
this(cause.getMessage(), cause);

@titusfortner
Copy link
Member Author

@diemol I updated the code to make sure getCause() isn't null. It doesn't matter if getMessage() is null, it will just end up in the same place, but if getCause() were null for some reason, then... NPE.

@titusfortner
Copy link
Member Author

@pujagani I wanted to keep the changes limited to the one place I saw a specific problem (missing info in the logs I'm looking at). The code is still the same in Java 21, which leads me to believe it is intentional? I raised a ticket with Oracle to get their response.

We will review your report and have assigned it an internal review ID : 9076067. Depending upon the completeness of the report and our ability to reproduce the problem, either a new bug will be posted, or we will contact you for further information.

🤷

@titusfortner titusfortner merged commit 33c4122 into trunk Oct 5, 2023
60 of 62 checks passed
@titusfortner titusfortner deleted the error_msg branch October 5, 2023 22:12
aguspe pushed a commit to aguspe/selenium that referenced this pull request Oct 22, 2023
* [java] ensure proper error message gets logged

* check getCause for null instead of getMessage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants