Skip to content

Conversation

@adeal
Copy link
Contributor

@adeal adeal commented May 16, 2025

What was changed

This a small change to make (most of) the ArgumentException messages in OptionsExtensions more verbose. Specifically, this change focuses on errors caused by the absence of a property being set, and includes the name of the options class that it is missing from.

Why?

The various ToInteropOptions(...) methods are the first place where options classes get checked for correctness. Since these are "deeper" in the SDK than what most users are digging into, it can make the error challenging to interpret.

For example, we can look at an error thrown when TargetHost is not set for HttpConnectProxyOptions:

image

  1. The call stack is unrelated to the location where a user might configure these client options, such as Program.cs or Startup.cs, so it can be unclear that this is a user error
  2. "TargetHost" is an overloaded property name throughout the SDK, so disambiguating it to HttpConnectProxyOptions would be helpful

Checklist

  1. Closes

  2. How was this tested:

Not necessary; no functional changes.

  1. Any docs updates needed?

None known.

@adeal adeal requested a review from a team as a code owner May 16, 2025 15:02
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Thanks!

@cretz cretz merged commit 910fb1c into temporalio:main May 16, 2025
9 checks passed
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