Skip to content

msOptionalToTs() drops Duration=0 (falsy guard bug) — silently ignores nextRetryDelay:0 on ApplicationFailure #2233

Description

@tsushanth

Bug

msOptionalToTs() in packages/common/src/time.ts (line 71) uses a falsy truthiness guard on a Duration parameter that can legally be the number 0:

export function msOptionalToTs(str: Duration | undefined | null): Timestamp | undefined {
  return str ? msToTs(str) : undefined;  // ← 0 is falsy → silently drops valid zero Duration
}

Duration includes number, and 0 is a valid duration meaning "zero milliseconds / no delay". Because 0 is falsy in JavaScript, msOptionalToTs(0) returns undefined instead of a zero Timestamp.

Impact

The most impactful call site is packages/common/src/converter/failure-converter.ts line 355:

nextRetryDelay: msOptionalToTs(err.nextRetryDelay),

If an ApplicationFailure is thrown with nextRetryDelay: 0 (intent: retry this activity immediately with no backoff), msOptionalToTs(0) returns undefined, the proto field is omitted, and the Temporal server falls back to normal exponential backoff — silently ignoring the user's override.

Inconsistency with sibling function

The sibling function msOptionalToNumber (line 74) already uses the correct explicit check:

export function msOptionalToNumber(val: Duration | undefined): number | undefined {
  if (val === undefined) return undefined;  // ← explicit check, handles 0 correctly
  return msToNumber(val);
}

Suggested fix

export function msOptionalToTs(str: Duration | undefined | null): Timestamp | undefined {
  return str != null ? msToTs(str) : undefined;
}

This matches the intent (skip only null/undefined) and handles 0 correctly.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions