Repository navigation
feat: add RequestTimeoutMiddleware - #146
Conversation
Add a per-request timeout middleware that runs chain.next() in a virtual thread via a singleton executor. Returns 504 Gateway Timeout (configurable) if the handler does not complete within the configured deadline. Equivalent to Hono's timeout() and Express's express-timeout. StructuredTaskScope was considered but remains a preview API in Java 25, so a singleton Executors.newVirtualThreadPerTaskExecutor() is used instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Change executor from static final to instance field so that close() can shut it down. This matches the MemoryStore pattern and enables graceful shutdown: in-flight virtual threads are allowed to complete before the executor terminates. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the executor-based implementation with StructuredTaskScope, which provides two improvements: 1. ScopedValue inheritance — fork() propagates scoped-value bindings to the subtask thread, unlike executor.submit() 2. Structured shutdown — scope.close() waits for the subtask to finish, eliminating the need for a separate Closeable lifecycle Timeout is configured via Configuration.withTimeout() rather than Future.get(timeout), making intent explicit. enkan-web is compiled with --enable-preview; applications must use --enable-preview at runtime only if they reference this class. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Document scope.close() blocking behavior after timeout in Javadoc - Validate timeoutStatus range (100-599) with MisconfigurationException - Add tests: invalid timeoutStatus, null response passthrough Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new web middleware to enforce a per-request processing deadline and introduces tests + build configuration needed to run it.
Changes:
- Introduce
RequestTimeoutMiddlewarethat returns a configurable timeout response when downstream processing exceeds a deadline. - Add JUnit coverage for normal completion, timeout behavior, custom timeout status, exception propagation, null response passthrough, and ScopedValue inheritance.
- Update
enkan-webMaven build to pass--enable-previewto compilation and test execution.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| enkan-web/src/main/java/enkan/middleware/RequestTimeoutMiddleware.java | Implements request timeout behavior using StructuredTaskScope, plus configurable timeout settings and response. |
| enkan-web/src/test/java/enkan/middleware/RequestTimeoutMiddlewareTest.java | Adds unit tests validating timeout and non-timeout behaviors (including ScopedValue behavior). |
| enkan-web/pom.xml | Enables Java preview features for compiling/running the module and its tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <configuration> | ||
| <argLine>--enable-preview</argLine> |
There was a problem hiding this comment.
The surefire here replaces (not appends to) the parent argLine (pom.xml:104-108). This drops the existing JVM flags (e.g., --add-opens and -XX:+EnableDynamicAgentLoading), which can cause module-specific test/runtime regressions. Consider appending --enable-preview to the inherited argLine instead of overwriting it.
| <argLine>--enable-preview</argLine> | |
| <argLine>${argLine} --enable-preview</ |
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <configuration> | ||
| <compilerArgs> | ||
| <arg>--enable-preview</arg> | ||
| </compilerArgs> | ||
| <testCompilerArgs> | ||
| <arg>--enable-preview</arg> | ||
| </testCompilerArgs> | ||
| </configuration> |
There was a problem hiding this comment.
Enabling --enable-preview for the main compilation in this module is a breaking operational change: class files produced by javac with preview enabled require the application JVM to be started with --enable-preview to load them. If the intent is to keep enkan-web usable without preview flags unless RequestTimeoutMiddleware is used, consider avoiding preview APIs (per the PR description) or isolating preview-dependent code into a separate artifact/module.
| /** | ||
| * Middleware that enforces a per-request timeout for downstream processing, | ||
| * using {@link StructuredTaskScope}. | ||
| * | ||
| * <p>The downstream chain runs in a virtual thread forked by | ||
| 8000 | * {@link StructuredTaskScope}. If the chain does not complete within the | |
| * configured timeout, the scope is cancelled and this middleware returns | ||
| * a {@code 504 Gateway Timeout} response (configurable via | ||
| * {@link #setTimeoutStatus(int)}). |
There was a problem hiding this comment.
This implementation uses StructuredTaskScope and requires --enable-preview, but the PR description states StructuredTaskScope is not used and that the middleware runs chain.next() via a singleton Executors.newVirtualThreadPerTaskExecutor(). Please align the implementation and PR description/design decision so reviewers/users aren’t misled about the chosen concurrency mechanism and preview requirements.
| * Applications that reference this class must also be <em>run</em> with | ||
| * {@code --enable-preview}; applications that do not reference it are | ||
| * unaffected. |
There was a problem hiding this comment.
The Javadoc says only applications that reference this class must be run with --enable-preview, but enabling preview compilation at the module level typically marks generated class files as preview and requires --enable-preview to load them at runtime. If you keep module-wide --enable-preview, this note should be updated to reflect that enkan-web consumers will need the flag even if they don’t use RequestTimeoutMiddleware.
| * Applications that reference this class must also be <em>run</em> with | |
| * {@code --enable-preview}; applications that do not reference it are | |
| * unaffected. | |
| * If enkan-web is built with module-wide preview enabled, applications that | |
| * load classes from that build must be <em>run</em> with | |
| * {@code --enable-preview}, even if they do not directly use this middleware. |
| throw new RuntimeException(cause); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| return timeoutResponse(); |
There was a problem hiding this comment.
InterruptedException here is treated as a timeout and converted to a 504 response. An interrupt can also indicate shutdown/cancellation unrelated to request deadline, and converting it to 504 can hide the interruption from upstream lifecycle handling. Consider propagating interruption (e.g., rethrow as unchecked after restoring interrupt status) or mapping it to a more appropriate response distinct from timeout.
| return timeoutResponse(); | |
| throw new RuntimeException("Request processing interrupted", e); |
- pom.xml: preserve parent argLine flags (--add-opens, -XX:+EnableDynamic AgentLoading) alongside --enable-preview instead of overwriting them - Javadoc: clarify that all enkan-web consumers need --enable-preview at runtime, not only users of RequestTimeoutMiddleware - InterruptedException: propagate as RuntimeException instead of mapping to a 504 timeout response, to avoid hiding shutdown/cancellation signals - Add test: interruptedExceptionPropagatesAsRuntimeException Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
RequestTimeoutMiddlewarethat enforces a per-request deadline usingStructuredTaskScopechain.next()in a virtual thread forked byStructuredTaskScope;ScopedValuebindings are inheritedsetTimeoutStatus(int)) if the handler exceedstimeoutMillis(default 30 s)timeout()middlewareDesign decisions
StructuredTaskScope(not singleton executor)ScopedValuebindings from the calling thread; structured shutdown ensures subtask cannot outlive the scope--enable-previewon enkan-webStructuredTaskScopeis a preview API in Java 25; all class files in the module are marked previewsetTimeoutStatus()ThreadLocal-based transaction context is not inherited; ordering guidance documentedTest plan
setTimeoutStatus(503)setTimeoutMillis(0)/ negative →MisconfigurationExceptionMisconfigurationExceptionInterruptedExceptionpropagates asRuntimeException(not swallowed as 504)ScopedValuebinding is inherited by the subtask🤖 Generated with Claude Code