Skip to content

Conversation

@roxblnfk
Copy link
Collaborator

@roxblnfk roxblnfk commented Oct 29, 2025

What was changed

Added warning when Carbon and native DateInterval parse ISO 8601 duration strings differently.

  • Added isIso8601DurationFormat() to validate ISO 8601 format
  • Modified parse() to compare Carbon vs DateInterval results and emit E_USER_WARNING on mismatch
  • Added tests for format detection and warning behavior

Why?

Carbon interprets P2M as "2 minutes" while ISO 8601 standard defines it as "2 months", causing incorrect timer durations. Warning suggests using new \DateInterval() for standard ISO 8601 parsing or adding T separator to clarify intent (e.g., PT2M for minutes).

Checklist

  1. Closes [DX] Unexpected behaviour with lax DateInterval string parsing #579
  2. How was this tested:
    added tests

DateInterval Ambiguity Warning

Added runtime detection for ISO 8601 duration strings where Carbon and native DateInterval produce different parse results.

The SDK internal duration parser uses Carbon's CarbonInterval::fromString() which interprets P2M as "2 minutes", while PHP's native DateInterval follows ISO 8601 standard and parses it as "2 months". This discrepancy causes incorrect timer durations.

When an ambiguous format is detected:

yield Workflow::timer('P2M');
// Warning: Ambiguous duration "P2M": Carbon and DateInterval parse it differently.
// Use new \DateInterval("P2M") for ISO 8601 standard parsing or PT/P prefix to clarify intent.

Resolution:

// Use T separator for time components (hours, minutes, seconds)
yield Workflow::timer('PT2M');  // 2 minutes

// Use natural language for Carbon parsing
yield Workflow::timer('2 minutes');  // 2 minutes
yield Workflow::timer('2 months');   // 2 months

// Use DateInterval instance for ISO 8601 compliance
yield Workflow::timer(new \DateInterval('P2M'));  // 2 months

Warning is triggered only when:

  1. Input string matches ISO 8601 duration format
  2. Carbon and DateInterval produce different interval values

Natural language formats (e.g., "2 days") are not validated against DateInterval and do not trigger warnings.

@roxblnfk roxblnfk requested a review from wolfy-j as a code owner October 29, 2025 11:27
@vercel
Copy link

vercel bot commented Oct 29, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
php Ready Ready Preview Comment Oct 30, 2025 1:26pm

@roxblnfk roxblnfk merged commit 8358594 into master Oct 31, 2025
244 of 267 checks passed
@roxblnfk roxblnfk deleted the warn-interval branch October 31, 2025 11:16
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.

[DX] Unexpected behaviour with lax DateInterval string parsing

3 participants