Skip to content

💥 feat(sdk): enforce some activity to close timeout is set#1221

Merged
chris-olszewski merged 6 commits into
masterfrom
olszewski/enforce_activity_timeouts
Apr 20, 2026
Merged

💥 feat(sdk): enforce some activity to close timeout is set#1221
chris-olszewski merged 6 commits into
masterfrom
olszewski/enforce_activity_timeouts

Conversation

@chris-olszewski

@chris-olszewski chris-olszewski commented Apr 17, 2026

Copy link
Copy Markdown
Member

What was changed

  • Move to a builder for ActivityOptions
  • Combine schedule_to_close_timeout and start_to_close_timeout into a combined field of close_timeouts: ActivityCloseTimeouts to enforce that some timeout to close is set.

Aside from the vanilla builder we add a few methods to help with ergonomics by letting them skip importing the new enum:

  • ActivityOptions::schedule_to_close_timeout/ActivityOptions::start_to_close_timeout: Directly construct activity options with just this timeout set
  • ActivityOptions::with_schedule_to_close_timeout/ActivityOptions::with_start_to_close_timeout: Start an options builder with one of these timeouts set
    There is no both variant for these since it would take 2 Duration objects and there isn't a clear way to indicate which one is for which timeout without a struct, which we already have this with the enum.

Why?

Collapsing the close timeouts allows us to enforce that all activities must have a timeout to close.

I ended up combining this with moving ActivityOptions to a builder as the close timeout change was invasive enough and at the end of the day I wanted to see the end state of how we want users to construct these options.

Alternatives Considered

There is a nice design where we continue to expose the activity timeout setting only via the builder with .start_to_close_timeout and schedule_to_close_timeout setters that both flow into a single required field.
elastio/bon#149

Checklist

  1. Closes N/A

  2. How was this tested:
    Migrated existing tests/examples to use new activity options API

  3. Any docs updates needed?
    README was updated

@chris-olszewski
chris-olszewski force-pushed the olszewski/enforce_activity_timeouts branch from 720d798 to 39801f1 Compare April 17, 2026 16:28
@chris-olszewski
chris-olszewski marked this pull request as ready for review April 17, 2026 18:53
@chris-olszewski
chris-olszewski requested a review from a team as a code owner April 17, 2026 18:53

@Sushisource Sushisource left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! This is a big improvement.

@chris-olszewski
chris-olszewski force-pushed the olszewski/enforce_activity_timeouts branch from e2990dd to 1891c4b Compare April 20, 2026 13:07
@chris-olszewski
chris-olszewski force-pushed the olszewski/enforce_activity_timeouts branch from ca4cded to 979ad37 Compare April 20, 2026 18:02
@chris-olszewski
chris-olszewski enabled auto-merge (squash) April 20, 2026 18:11
@chris-olszewski
chris-olszewski merged commit 9171aca into master Apr 20, 2026
21 checks passed
@chris-olszewski
chris-olszewski deleted the olszewski/enforce_activity_timeouts branch April 20, 2026 18:14
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