8371091: Improve the exception message of NullPointerException thrown by the methods in the default implementation of HttpRequest.Builder #28103

ehs208 · 2025-11-02T07:19:12Z

This change adds clearer NullPointerException messages to the POST(), PUT(), and method() methods in HttpRequest.Builder when the provided BodyPublisher is null.

Each of these methods now calls requireNonNull(body, "BodyPublisher must not be null"), making the exception message explicit.
A corresponding regression test (RequestBuilderNullBodyTest) has been added to verify this behavior.

All builds and related tests (jdk/java/net/httpclient) have passed successfully.

Currently, only POST(), PUT(), and method() include these messages.
Some other builder methods (such as header(), setHeader()) also throw NullPointerException without a message.
I would like to discuss whether adding consistent messages to those methods would be beneficial,
and whether the current wording "BodyPublisher must not be null" feels appropriate.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8371091: Improve the exception message of NullPointerException thrown by the methods in the default implementation of HttpRequest.Builder (Enhancement - P4)

Reviewers

Daniel Fuchs (@dfuch - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28103/head:pull/28103
$ git checkout pull/28103

Update a local copy of the PR:
$ git checkout pull/28103
$ git pull https://git.openjdk.org/jdk.git pull/28103/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28103

View PR using the GUI difftool:
$ git pr show -t 28103

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28103.diff

Using Webrev

Link to Webrev Comment

…ublisher in HttpRequest.Builder methods

…dyPublisher in HttpRequest.Builder

bridgekeeper · 2025-11-02T07:20:11Z

👋 Welcome back ehs208! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-11-02T07:21:29Z

@ehs208 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8371091: Improve the exception message of NullPointerException thrown by the methods in the default implementation of HttpRequest.Builder

Reviewed-by: dfuchs

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 86 new commits pushed to the master branch:

a586458: 8155591: Misleading warning when not overriding close method in interface extending AutoCloseable
2d924ad: 8351194: Clean up Hotspot SA after 32-bit x86 removal
df414e0: 8370884: JFR: Overflow in aggregators
... and 83 more: https://git.openjdk.org/jdk/compare/7c900da1985c9508198ea7805e4955da4a7cac42...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@dfuch) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2025-11-02T07:22:20Z

@ehs208 The following label will be automatically applied to this pull request:

net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-11-02T07:30:40Z

Webrevs

dfuch · 2025-11-03T12:05:31Z

src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestBuilderImpl.java


    @Override
    public HttpRequest.Builder method(String method, BodyPublisher body) {
        requireNonNull(method);


It seems strange to improve the message for body but not for method.

I agree for consistency.
I'll update the requireNonNull(method) call to include an explicit message as well, e.g.,
requireNonNull(method, "HTTP method must be non-null").

jaikiran · 2025-11-03T12:31:45Z

Hello @ehs208, instead of a new test class, could you see if the existing tests in test/jdk/java/net/httpclient/HttpRequestBuilderTest.java can be updated for additionally checking the NullPointerException message wherever applicable?

ehs208 · 2025-11-03T12:40:27Z

Hello @ehs208, instead of a new test class, could you see if the existing tests in test/jdk/java/net/httpclient/HttpRequestBuilderTest.java can be updated for additionally checking the NullPointerException message wherever applicable?

Thanks for the suggestion! That makes sense — instead of keeping a separate test class,
I can move these checks into HttpRequestBuilderTest.java.

However, since the existing testNull() method focuses only on verifying that NPEs are thrown
(regardless of message content), would it make sense to add a small new test method,
say testNullMessages(), in the same class to verify the exception messages specifically?

That way we keep the tests co-located but with a clear separation of intent.

jaikiran · 2025-11-03T16:13:17Z

would it make sense to add a small new test method,
say testNullMessages(), in the same class to verify the exception messages specifically?

I think that should be OK. It doesn't have to be too complicated/generic either, as long as it tests these specific method calls.

…tespaces error

ehs208 · 2025-11-04T11:33:03Z

would it make sense to add a small new test method,
say testNullMessages(), in the same class to verify the exception messages specifically?

I think that should be OK. It doesn't have to be too complicated/generic either, as long as it tests these specific method calls.

Thanks for confirming! I’ll add a straightforward testNullMessages() method in the same class to verify those specific calls.

dfuch

I ran HttpClient tests in the CI and they came back green.
So look good to me!

ehs208 · 2025-11-06T15:53:25Z

/integrate

openjdk · 2025-11-06T15:54:18Z

@ehs208
Your change (at version f26cc33) is now ready to be sponsored by a Committer.

dfuch · 2025-11-06T16:11:37Z

/sponsor

openjdk · 2025-11-06T16:13:39Z

Going to push as commit c272aca.
Since your change was applied there have been 88 commits pushed to the master branch:

1f08a3e: 8355342: File.getCanonicalPath on Java 24 resolves paths on network drives to UNC format
1321186: 8367943: PipedOutputStream write(0, 0) successful after close()
a586458: 8155591: Misleading warning when not overriding close method in interface extending AutoCloseable
... and 85 more: https://git.openjdk.org/jdk/compare/7c900da1985c9508198ea7805e4955da4a7cac42...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-11-06T16:13:56Z

@dfuch @ehs208 Pushed as commit c272aca.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

ehs208 added 2 commits November 2, 2025 16:03

8271091: Add descriptive NullPointerException messages for null BodyP…

d3d8554

…ublisher in HttpRequest.Builder methods

8271091: Add test verifying NullPointerException messages for null Bo…

3185190

…dyPublisher in HttpRequest.Builder

openjdk bot added the net net-dev@openjdk.org label Nov 2, 2025

8271091: fixed whitespace errors

169294d

openjdk bot added the rfr Pull request is ready for review label Nov 2, 2025

ehs208 changed the title ~~8371091: Enhance npe message httprequest~~ 8371091: Improve the exception message of NullPointerException thrown by the methods in the default implementation of HttpRequest.Builder Nov 2, 2025

dfuch reviewed Nov 3, 2025

View reviewed changes

8271091: Add NPE messages for method

24ecfc3

8371091: Add testNullMessages() to HttpRequestBuilderTest

b90837b

openjdk bot removed the rfr Pull request is ready for review label Nov 4, 2025

8371091: Add testNullMessages() to HttpRequestBuilderTest and fix whi…

f26cc33

…tespaces error

openjdk bot added the rfr Pull request is ready for review label Nov 4, 2025

ehs208 requested a review from dfuch November 6, 2025 11:18

dfuch approved these changes Nov 6, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 6, 2025

openjdk bot added the sponsor Pull request is ready to be sponsored label Nov 6, 2025

openjdk bot added the integrated Pull request has been integrated label Nov 6, 2025

openjdk bot closed this Nov 6, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Nov 6, 2025

8371091: Improve the exception message of NullPointerException thrown by the methods in the default implementation of HttpRequest.Builder #28103

8371091: Improve the exception message of NullPointerException thrown by the methods in the default implementation of HttpRequest.Builder #28103

Uh oh!

Conversation

ehs208 commented Nov 2, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Nov 2, 2025

Uh oh!

openjdk bot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

dfuch Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

ehs208 Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

jaikiran commented Nov 3, 2025

Uh oh!

ehs208 commented Nov 3, 2025

Uh oh!

jaikiran commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ehs208 commented Nov 4, 2025

Uh oh!

dfuch left a comment

Choose a reason for hiding this comment

Uh oh!

ehs208 commented Nov 6, 2025

Uh oh!

openjdk bot commented Nov 6, 2025

Uh oh!

dfuch commented Nov 6, 2025

Uh oh!

openjdk bot commented Nov 6, 2025

Uh oh!

openjdk bot commented Nov 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

ehs208 commented Nov 2, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 2, 2025 •

edited

Loading

openjdk bot commented Nov 2, 2025 •

edited

Loading

mlbridge bot commented Nov 2, 2025 •

edited

Loading

jaikiran commented Nov 3, 2025 •

edited

Loading