Skip to content

refactor(sdk/go): unify nil handling and apply-loop pattern for functional options #3213

Description

@rhuss

Description

The Go SDK has 14 functional option types applied at 16 call sites across six
packages. Only one of them, ApplyCreateOptions, guards against a nil option.
Every other apply loop calls opt(&cfg) unconditionally and panics with a nil
pointer dereference if a caller passes a nil option.

Two things should be decided and then applied consistently:

  1. Nil policy. Either ignore nil options everywhere, or document that a nil
    option is a programming error and let it panic. Right now the SDK does one
    thing in Create and the opposite everywhere else, which is the worst of
    both options because callers cannot rely on either behavior.

  2. Apply-loop duplication. Six Apply*Options helpers live in types/,
    while oidc, fake, gateway, edge, and the v1 transport clients each
    hand-roll the same loop inline. A single generic helper would collapse all of
    them and make the nil policy a one-line decision instead of a 16-site edit.

Context

This came out of review on #3198, which replaced the positional labels
parameter on Create() and CreateFromTemplate() with functional options.
Because the removed parameter sat immediately before the variadic, existing
calls like Create(ctx, ws, name, spec, nil) still compiled and began panicking
at runtime. That was fixed in #3198 by nil-guarding ApplyCreateOptions only,
since it was the one type with an actual migration hazard.

The remaining types have no such hazard, so there is no urgency here. This is a
consistency and API-design question rather than a bug.

Current state

Location Applier Nil-guarded
types/options.go:34 ApplyCreateOptions yes
types/log.go:72 ApplyLogOptions no
types/policy.go:267,304,348,400 ApplyGetDraftOptions, ApplyApproveAllOptions, ApplyGetStatusOptions, ApplyListPolicyOptions no
oidc/oidc.go:36, oidc/device.go:49, oidc/credentials.go:40 inline no
fake/fake.go:100 inline no
gateway/gateway.go:30 inline no
ssh_client.go:67, tcp_client.go:46,106 inline no
edge/tunnel.go:110, auth_refresh.go:163 inline no

Definition of Done

  • A single nil policy is chosen and recorded in the SDK docs
  • The policy is applied uniformly across all 16 apply sites
  • Duplicated apply loops are consolidated behind a shared helper, or a note
    explains why per-package loops are kept
  • Tests cover the chosen behavior for at least one option type per package

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    state:triage-neededOpened without agent diagnostics and needs triage

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions