Skip to content

Conversation

@stephanos
Copy link
Contributor

@stephanos stephanos commented Oct 8, 2025

What changed?

Replace a few serviceerror.NewInternal with softassert.UnexpectedInternalErr.

Why?

Catch these issues in CICD and other testing.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@stephanos stephanos changed the title Add softassert.UnexpectedInternalErr Add more softassert.UnexpectedInternalErr Oct 8, 2025
@stephanos stephanos force-pushed the internal-assertions branch from 9daf1da to d6bc337 Compare October 8, 2025 23:00
@stephanos stephanos marked this pull request as ready for review October 30, 2025 16:40
@stephanos stephanos requested review from a team as code owners October 30, 2025 16:40
@stephanos stephanos requested a review from spkane31 October 30, 2025 16:41
if s.requestEagerStart() && workflowTaskInfo == nil {
return nil, serviceerror.NewInternal("unexpected error: mutable state did not have a started workflow task")
return nil, softassert.UnexpectedInternalErr(
s.shardContext.GetLogger(),
Copy link
Contributor

Choose a reason for hiding this comment

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

only comment, do we rely on error strings anywhere (we shouldn't imo) but if so we shouldn't change this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so; but I can add it back, doesn't hurt.

@stephanos stephanos enabled auto-merge (squash) October 31, 2025 15:10
@stephanos stephanos merged commit 3c51318 into temporalio:main Oct 31, 2025
57 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