Skip to content

Refactor signer name consistency enforcement in AppendOptions validation#958

Merged
roger2hk merged 1 commit into
transparency-dev:mainfrom
roger2hk:slog-ctx
May 13, 2026
Merged

Refactor signer name consistency enforcement in AppendOptions validation#958
roger2hk merged 1 commit into
transparency-dev:mainfrom
roger2hk:slog-ctx

Conversation

@roger2hk
Copy link
Copy Markdown
Contributor

@roger2hk roger2hk commented May 13, 2026

This pull request defers the signer validation in WithCheckpointSigner to avoid os.Exit and context.Background.

Problem
The WithCheckpointSigner method was performing immediate validation on the provided signers. However, this method is a configuration setter and does not take a context.Context. Only context.Background() can be used to log errors when validation failed. Furthermore, it called os.Exit(1) directly upon failure, which is anti-idiomatic for library code as it prevents the calling application from handling the failure gracefully.

Solution
This change defers the validation of additional signers to the valid() method of AppendOptions. If validation fails, valid() returns an error instead of crashing the process. This approach does not cause any breaking changes.

Why better?

  • Idiomatic Go Error Handling: Libraries should return errors rather than calling os.Exit. This allows the application using Tessera to decide how to handle invalid configurations.
  • Eliminates dummy contexts: By moving the check to valid() (which is called by NewAppender in a context-aware environment), we no longer need to use context.Background() for logging errors.

@roger2hk roger2hk requested a review from mhutchinson May 13, 2026 10:19
@roger2hk roger2hk requested a review from a team as a code owner May 13, 2026 10:19
@AlCutter
Copy link
Copy Markdown
Collaborator

Could you add a description which explains why this change is necessary/better?

@roger2hk
Copy link
Copy Markdown
Contributor Author

Could you add a description which explains why this change is necessary/better?

Sure, I've just updated the pull request description.

@roger2hk roger2hk merged commit 12a447b into transparency-dev:main May 13, 2026
15 checks passed
@roger2hk roger2hk deleted the slog-ctx branch May 13, 2026 13:58
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.

3 participants