Skip to content

Add docs.nats.io examples to main - #1600

Merged
Jarema merged 2 commits into
mainfrom
docs-examples-to-main
Jul 21, 2026
Merged

Add docs.nats.io examples to main#1600
Jarema merged 2 commits into
mainfrom
docs-examples-to-main

Conversation

@Jarema

@Jarema Jarema commented Jul 21, 2026

Copy link
Copy Markdown
Member

Moves the documentation examples from the core-docs and jetstream-docs branches into examples/src/main/java/io/nats/examples/natsIoDoc/ on main (52 new Learn*.java files joining the 19 already here; additive only).

Why: the new docs.nats.io build fetches these snippets at build time. On side branches they get zero CI (no workflow triggers on them); on main the examples subproject's compileJava covers them on every PR/push automatically — no gradle or workflow changes needed.

Note: please keep the docs branches until the docs repo's fetch config is switched to main (follow-up PR there).

🤖 Generated with Claude Code

@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @Jarema's task in 3m 14s —— View job


PR Review

  • Gather context and load tools
  • Read CLAUDE.md for repo guidelines (none found)
  • Get PR diff and examine files
  • Review Core NATS examples
  • Review JetStream examples (exception handling, resource cleanup, API correctness)
  • Post inline comments for specific issues
  • Write final review summary

The 52 examples are well-structured and consistent. All connections are properly closed via try-with-resources, JetStream FetchConsumers are all wrapped correctly, ordered consumers are used appropriately (no ack), and the consume()/MessageConsumer pattern for continuous pull is handled correctly.

Three issues worth addressing:

1. Missing null-check → NPE (medium)LearnJetStreamRetentionPoliciesWorkQueueCreate.java:52 calls fc.nextMessage() without checking for null before accessing task.getData(). Every other nextMessage() call in the PR guards against null. See inline comment.

2. JetStreamStatusException in a Core NATS example (verify)LearnCoreNatsRequestReplyRequest.java:55 catches JetStreamStatusException to detect the "no responders" 503 status on a plain nc.request() call. The class name implies JetStream but this is a Core NATS feature. Please confirm this is the exception the library actually throws here (vs. a StatusException base class); readers who notice the import may be confused. See inline comment.

3. Silent longint narrowing cast (minor)LearnJetStreamReadingBackRead.java:25 casts getNumPending() (a long) to int for maxMessages(). Pathologically large pending counts wrap to a negative value. See inline comment with suggestion.

Two patterns that are fine but worth being aware of:

  • The empty catch (IOException e) bodies in Core NATS examples (e.g. LearnCoreNatsPublishSubscribePublish.java:28) are intentional for brevity and the comment explains why, but they silence connect errors completely. Readers who copy the pattern into production code won't see any connection-failure output.
  • LearnJetStreamWorkerPoolRedeliveryCount.java calls cc.fetch(FetchConsumeOptions.builder().maxMessages(10).build()) without an explicit expiresIn, unlike every other fetch example in the PR. This will use the library's default timeout, which is probably fine, but the inconsistency could confuse readers comparing examples.

System.out.println("Messages remaining in FULFILLMENT: " + count);
// NATS-DOC-END
}
catch (Exception e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing null-check before using task. fc.nextMessage() returns null when the fetch times out with no messages delivered. Every other nextMessage() call in this PR checks for null first — e.g. LearnJetStreamYourFirstConsumerPullAndAck.java:16. If there's a transient delay between the publish and the fetch (e.g. a slow CI worker), this will throw NullPointerException on task.getData().

Suggested change
catch (Exception e) {
try (FetchConsumer fc = shippers.fetchMessages(1)) {
Message task = fc.nextMessage();
if (task == null) {
System.out.println("No task available.");
return;
}
System.out.println("Shipping: " + new String(task.getData(), StandardCharsets.UTF_8));
task.ack();
}

}
else {
System.out.println("request failed: " + e.getMessage());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JetStreamStatusException is a JetStream-specific type but this is a Core NATS request. The "no responders" 503 status from reportNoResponders() is a Core NATS protocol feature. Please verify that the library does in fact complete the future with a JetStreamStatusException for this case (as opposed to a StatusException or similar base class). If readers copy this import and class name into a pure Core NATS project they may be confused by the JetStream dependency.

}

try (FetchConsumer fc = cc.fetch(
FetchConsumeOptions.builder().maxMessages((int) pending).build())) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Silent narrowing cast: getNumPending() returns long but maxMessages() takes int. Values above Integer.MAX_VALUE (~2.1B messages) wrap silently to a negative number, which would cause the fetch to fail. For a docs example this is extremely unlikely in practice, but since the code reads like it calculates a precise count it's worth clamping:

Suggested change
FetchConsumeOptions.builder().maxMessages((int) pending).build())) {
FetchConsumeOptions.builder().maxMessages((int) Math.min(pending, Integer.MAX_VALUE)).build())) {

@scottf scottf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Jarema
Jarema force-pushed the docs-examples-to-main branch from 1c23fb0 to b9deef3 Compare July 21, 2026 18:01
Jarema and others added 2 commits July 21, 2026 20:02
Consolidates the core-docs and jetstream-docs branches' examples
(examples/src/main/java/io/nats/examples/natsIoDoc/) onto main so the
documentation examples are compiled by CI (:examples:compileJava)
instead of living on unprotected side branches.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@synadia.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@synadia.com>
@Jarema
Jarema force-pushed the docs-examples-to-main branch from b9deef3 to 8cb8a91 Compare July 21, 2026 18:02
@Jarema
Jarema merged commit 95336bf into main Jul 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants