fix(wait-task): accept ISO-8601 duration and fix misleading error message - #1474
fix(wait-task): accept ISO-8601 duration and fix misleading error message#1474shaileshpadave wants to merge 3 commits into
Conversation
…nvalid format parseDuration() only accepted the custom "1s"/"2m"/"1h 30m" format. Using ISO-8601 (e.g. "PT1S") threw an IllegalArgumentException that got caught and re-thrown with the misleading "Either date or duration is passed as null" message. Now parseDuration() tries the custom format first, then falls back to Duration.parse() for ISO-8601. If both fail, the constraint surfaces the actual error message from parseDuration() instead of the null-blame message. Fixes #1310
| return Duration.parse(text); | ||
| } catch (java.time.format.DateTimeParseException e) { | ||
| throw new IllegalArgumentException("Not valid duration: " + text); | ||
| } |
There was a problem hiding this comment.
Context
This looks good. Accepting ISO 8601 durations through the Duration.parse fallback is clean, the regex and ISO paths do not overlap, and the new error message is a real improvement over the old date or duration is null wording. I also checked the message change at WorkflowTaskTypeConstraint.java:333 that passes e.getMessage() with the user text into buildConstraintViolationWithTemplate, and on Hibernate Validator 8 custom violation messages do not evaluate EL and an unbalanced brace only logs a warning, so there is no injection or interpolation crash there.
▎ Example: The Duration.parse fallback now accepts values the old regex never could, like a negative duration PT-1H or a zero or sub second duration PT0S or PT0.5S. The duration branch in WaitTaskMapper at 100 to 106 does not floor the result the way the until branch does at 114 to 117, so a negative duration sets callbackAfterSeconds to a negative value and a waitTimeout in the past, and the WAIT completes immediately instead of waiting.
Suggestion
Flooring the computed seconds at zero in the duration branch, or rejecting non positive durations in parseDuration, would keep the two paths consistent. A couple of tests for the negative and zero and sub second cases, plus one assertion on the new constraint message, would round it out since the added tests only cover the happy path values.
NOTE
None of this blocks the fix.
3 participants
What
A WAIT task with
"duration": "PT1S"(ISO-8601) failed with "Either date or duration is passed as null" — a message that implies the field was absent, when it was actually present but in an unsupported format.Two changes:
DateTimeUtils.parseDuration()— after the custom format ("1s","2m") fails to match, fall back tojava.time.Duration.parse()so ISO-8601 durations like"PT1S","P5D","PT1H30M"work natively.WorkflowTaskTypeConstraint— whenparseDuration()still throws (genuinely invalid input), surface the actual error message instead of the misleading "passed as null" message.Fixes #1310
How to verify
{ "type": "WAIT", "name": "wait1", "taskReferenceName": "wait_ref", "inputParameters": { "duration": "PT1S" } }"duration": "PT"— the error now reads "Not valid duration: PT" instead of blaming null.