Skip to content

Commit 917c8bc

Browse files
authored
Add test that repros TS crash due to proto changes (temporalio#423)
1 parent 69ca251 commit 917c8bc

File tree

15 files changed

+241
-32
lines changed

15 files changed

+241
-32
lines changed
+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Basic activity
2+
The most basic workflow which just runs an activity and returns its result.
3+
Importantly, without setting a workflow execution timeout.
4+
5+
6+
# Detailed spec
7+
It's important that the workflow execution timeout is not set here, because server will propagate that to all un-set
8+
activity timeouts. We had a bug where TS would crash (after proto changes from gogo to google) because it was expecting
9+
timeouts to be set to zero rather than null.
+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
namespace activity.basic_no_workflow_timeout;
2+
3+
using Temporalio.Activities;
4+
using Temporalio.Client;
5+
using Temporalio.Exceptions;
6+
using Temporalio.Features.Harness;
7+
using Temporalio.Worker;
8+
using Temporalio.Workflows;
9+
10+
class Feature : IFeature
11+
{
12+
public void ConfigureWorker(Runner runner, TemporalWorkerOptions options) =>
13+
options.AddWorkflow<MyWorkflow>().AddAllActivities(new MyActivities(runner.Client));
14+
15+
[Workflow]
16+
class MyWorkflow
17+
{
18+
private string? activityResult;
19+
20+
[WorkflowRun]
21+
public async Task RunAsync()
22+
{
23+
await Workflow.ExecuteActivityAsync(
24+
(MyActivities act) => act.Echo(),
25+
new()
26+
{
27+
ScheduleToCloseTimeout = TimeSpan.FromMinutes(1)
28+
});
29+
30+
await Workflow.ExecuteActivityAsync(
31+
(MyActivities act) => act.Echo(),
32+
new()
33+
{
34+
StartToCloseTimeout = TimeSpan.FromMinutes(1)
35+
});
36+
}
37+
38+
[WorkflowSignal]
39+
public async Task SetActivityResultAsync(string res) => activityResult = res;
40+
}
41+
42+
class MyActivities
43+
{
44+
private readonly ITemporalClient client;
45+
46+
public MyActivities(ITemporalClient client) => this.client = client;
47+
48+
[Activity]
49+
public async Task<string> Echo()
50+
{
51+
return "hi";
52+
}
53+
}
54+
}
+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package retry_on_error
2+
3+
import (
4+
"context"
5+
"time"
6+
7+
"github.com/temporalio/features/harness/go/harness"
8+
"go.temporal.io/sdk/client"
9+
"go.temporal.io/sdk/workflow"
10+
)
11+
12+
var Feature = harness.Feature{
13+
Workflows: Workflow,
14+
Activities: Echo,
15+
StartWorkflowOptionsMutator: func(o *client.StartWorkflowOptions) {
16+
o.WorkflowExecutionTimeout = 0
17+
},
18+
}
19+
20+
func Workflow(ctx workflow.Context) (string, error) {
21+
ctx = workflow.WithActivityOptions(ctx, workflow.ActivityOptions{
22+
StartToCloseTimeout: 1 * time.Minute,
23+
})
24+
25+
var result string
26+
err := workflow.ExecuteActivity(ctx, Echo).Get(ctx, &result)
27+
if err != nil {
28+
return "", err
29+
}
30+
31+
ctx = workflow.WithActivityOptions(ctx, workflow.ActivityOptions{
32+
ScheduleToCloseTimeout: 1 * time.Minute,
33+
})
34+
err = workflow.ExecuteActivity(ctx, Echo).Get(ctx, &result)
35+
return result, err
36+
}
37+
38+
func Echo(_ context.Context) (string, error) {
39+
return "echo", nil
40+
}
+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package activity.basic_no_workflow_timeout;
2+
3+
import io.temporal.activity.ActivityInterface;
4+
import io.temporal.activity.ActivityMethod;
5+
import io.temporal.client.WorkflowOptions;
6+
import io.temporal.sdkfeatures.Feature;
7+
import io.temporal.sdkfeatures.SimpleWorkflow;
8+
import java.time.Duration;
9+
10+
@ActivityInterface
11+
public interface feature extends Feature, SimpleWorkflow {
12+
@ActivityMethod
13+
String echo();
14+
15+
class Impl implements feature {
16+
@Override
17+
public void workflow() {
18+
var activities =
19+
activities(
20+
feature.class, builder -> builder.setStartToCloseTimeout(Duration.ofMinutes(1)));
21+
22+
activities.echo();
23+
24+
var activitiesSched2Close =
25+
activities(
26+
feature.class, builder -> builder.setScheduleToCloseTimeout(Duration.ofMinutes(1)));
27+
28+
activitiesSched2Close.echo();
29+
}
30+
31+
@Override
32+
public String echo() {
33+
return "hi";
34+
}
35+
}
36+
37+
@Override
38+
default void workflowOptions(WorkflowOptions.Builder builder) {
39+
builder.setWorkflowExecutionTimeout(Duration.ZERO);
40+
}
41+
}
+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
from datetime import timedelta
2+
3+
from temporalio import activity, workflow
4+
5+
from harness.python.feature import register_feature
6+
7+
8+
@workflow.defn
9+
class Workflow:
10+
@workflow.run
11+
async def run(self) -> str:
12+
await workflow.execute_activity(
13+
echo,
14+
schedule_to_close_timeout=timedelta(minutes=1),
15+
)
16+
return await workflow.execute_activity(
17+
echo,
18+
start_to_close_timeout=timedelta(minutes=1),
19+
)
20+
21+
22+
@activity.defn
23+
async def echo() -> str:
24+
return "echo"
25+
26+
27+
register_feature(
28+
workflows=[Workflow],
29+
activities=[echo],
30+
expect_activity_error="activity attempt 5 failed",
31+
start_options={"execution_timeout": None},
32+
)
+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { Feature } from '@temporalio/harness';
2+
import * as wf from '@temporalio/workflow';
3+
4+
const activities = wf.proxyActivities<typeof activitiesImpl>({
5+
startToCloseTimeout: '1 minute',
6+
});
7+
const activitiesSched2Close = wf.proxyActivities<typeof activitiesImpl>({
8+
scheduleToCloseTimeout: '1 minute',
9+
});
10+
11+
export async function workflow(): Promise<string> {
12+
await activitiesSched2Close.echo('hello');
13+
return await activities.echo('hello');
14+
}
15+
16+
const activitiesImpl = {
17+
async echo(input: string): Promise<string> {
18+
return input;
19+
},
20+
};
21+
22+
export const feature = new Feature({
23+
workflow,
24+
workflowStartOptions: { workflowExecutionTimeout: undefined },
25+
activities: activitiesImpl,
26+
});

Diff for: features/eager_workflow/successful_start/feature.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ const expectedResult = "Hello World"
1818
var numEagerlyStarted atomic.Uint64
1919

2020
var Feature = harness.Feature{
21-
Workflows: Workflow,
22-
StartWorkflowOptions: client.StartWorkflowOptions{EnableEagerStart: true, WorkflowTaskTimeout: 1 * time.Hour},
23-
CheckResult: CheckResult,
21+
Workflows: Workflow,
22+
StartWorkflowOptionsMutator: func(o *client.StartWorkflowOptions) {
23+
o.EnableEagerStart = true
24+
o.WorkflowTaskTimeout = 1 * time.Hour
25+
},
26+
CheckResult: CheckResult,
2427
ClientOptions: client.Options{
2528
ConnectionOptions: client.ConnectionOptions{
2629
DialOptions: []grpc.DialOption{grpc.WithUnaryInterceptor(EagerDetector(&numEagerlyStarted))},

Diff for: features/features.go

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package features
22

33
import (
4+
activity_basic_no_workflow_timeout "github.com/temporalio/features/features/activity/basic_no_workflow_timeout"
45
activity_cancel_try_cancel "github.com/temporalio/features/features/activity/cancel_try_cancel"
56
activity_retry_on_error "github.com/temporalio/features/features/activity/retry_on_error"
67
bugs_go_activity_start_race "github.com/temporalio/features/features/bugs/go/activity_start_race"
@@ -52,6 +53,7 @@ import (
5253
func init() {
5354
// Please keep list in alphabetical order
5455
harness.MustRegisterFeatures(
56+
activity_basic_no_workflow_timeout.Feature,
5557
activity_cancel_try_cancel.Feature,
5658
activity_retry_on_error.Feature,
5759
bugs_go_activity_start_race.Feature,

Diff for: features/schedule/cron/feature.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ import (
1313
)
1414

1515
var Feature = harness.Feature{
16-
Workflows: Workflow,
17-
StartWorkflowOptions: client.StartWorkflowOptions{CronSchedule: "@every 2s"},
18-
CheckResult: CheckResult,
16+
Workflows: Workflow,
17+
StartWorkflowOptionsMutator: func(o *client.StartWorkflowOptions) {
18+
o.CronSchedule = "@every 2s"
19+
},
20+
CheckResult: CheckResult,
1921
// Disable history check because we can't guarantee cron execution times
2022
CheckHistory: harness.NoHistoryCheck,
2123
}

Diff for: features/update/self/feature.go

+4-6
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,11 @@ var Feature = harness.Feature{
3333
if reason := updateutil.CheckServerSupportsUpdate(ctx, runner.Client); reason != "" {
3434
return nil, runner.Skip(reason)
3535
}
36-
opts := runner.Feature.StartWorkflowOptions
37-
if opts.TaskQueue == "" {
38-
opts.TaskQueue = runner.TaskQueue
39-
}
40-
if opts.WorkflowExecutionTimeout == 0 {
41-
opts.WorkflowExecutionTimeout = 1 * time.Minute
36+
opts := client.StartWorkflowOptions{
37+
TaskQueue: runner.TaskQueue,
38+
WorkflowExecutionTimeout: 1 * time.Minute,
4239
}
40+
runner.Feature.StartWorkflowOptionsMutator(&opts)
4341
return runner.Client.ExecuteWorkflow(ctx, opts, SelfUpdateWorkflow, ConnMaterial{
4442
HostPort: runner.Feature.ClientOptions.HostPort,
4543
Namespace: runner.Feature.ClientOptions.Namespace,

Diff for: harness/go/cmd/run.go

+5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"go.temporal.io/sdk/client"
78
"io"
89
"net"
910
"net/url"
@@ -204,6 +205,10 @@ func (r *Runner) Run(ctx context.Context, run *Run) error {
204205
return nil
205206
}
206207

208+
if feature.StartWorkflowOptionsMutator == nil {
209+
feature.StartWorkflowOptionsMutator = func(opts *client.StartWorkflowOptions) {}
210+
}
211+
207212
runnerConfig := harness.RunnerConfig{
208213
ServerHostPort: r.config.Server,
209214
Namespace: r.config.Namespace,

Diff for: harness/go/harness/feature.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,16 @@ type Feature struct {
4141
// overridden internally.
4242
ClientOptions client.Options
4343

44-
// Worker options for worker creation. Some values like WorkflowPanicPolicy
45-
// are always overridden internally.
44+
// Worker options for worker creation. Some values like WorkflowPanicPolicy are always
45+
// overridden internally. By default, the harness sets the WorkflowPanicPolicy to FailWorkflow -
46+
// in order to set that one option here you must *also* set the
47+
// DisableWorkflowPanicPolicyOverride field to true.
4648
WorkerOptions worker.Options
4749

48-
// Start workflow options that are used by the default executor. Some values
49-
// such as task queue and workflow execution timeout, are set by default if
50-
// not already set. By default the harness sets the WorkflowPanicPolicy to
51-
// FailWorkflow - in order to set that one option here you must *also* set the
52-
// DisableWorkflowPanicPolicyOverride field to true.
53-
StartWorkflowOptions client.StartWorkflowOptions
50+
// Can modify the workflow options that are used by the default executor. Some values such as
51+
// task queue and workflow execution timeout, are set by default (but may be overridden by this
52+
// mutator).
53+
StartWorkflowOptionsMutator func(*client.StartWorkflowOptions)
5454

5555
// The harness will override the WorkflowPanicPolicy to be FailWorkflow
5656
// unless this field is set to true, in which case the WorkflowPanicPolicy

Diff for: harness/go/harness/runner.go

+4-6
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,11 @@ func (r *Runner) Run(ctx context.Context) error {
149149
// ExecuteDefault is the default execution that just runs the first workflow and
150150
// assumes it takes no parameters.
151151
func (r *Runner) ExecuteDefault(ctx context.Context) (client.WorkflowRun, error) {
152-
opts := r.Feature.StartWorkflowOptions
153-
if opts.TaskQueue == "" {
154-
opts.TaskQueue = r.TaskQueue
155-
}
156-
if opts.WorkflowExecutionTimeout == 0 {
157-
opts.WorkflowExecutionTimeout = 1 * time.Minute
152+
opts := client.StartWorkflowOptions{
153+
TaskQueue: r.TaskQueue,
154+
WorkflowExecutionTimeout: 1 * time.Minute,
158155
}
156+
r.Feature.StartWorkflowOptionsMutator(&opts)
159157
firstWorkflow, err := r.Feature.GetPrimaryWorkflow()
160158
if err != nil {
161159
return nil, err

Diff for: harness/go/harness/util.go

+4-6
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,11 @@ func FindEvent(history client.HistoryEventIterator, cond func(*historypb.History
4545
// ExecuteWithArgs runs a workflow with default arguments
4646
func ExecuteWithArgs(workflow interface{}, args ...interface{}) func(ctx context.Context, r *Runner) (client.WorkflowRun, error) {
4747
return func(ctx context.Context, r *Runner) (client.WorkflowRun, error) {
48-
opts := r.Feature.StartWorkflowOptions
49-
if opts.TaskQueue == "" {
50-
opts.TaskQueue = r.TaskQueue
51-
}
52-
if opts.WorkflowExecutionTimeout == 0 {
53-
opts.WorkflowExecutionTimeout = 1 * time.Minute
48+
opts := client.StartWorkflowOptions{
49+
TaskQueue: r.TaskQueue,
50+
WorkflowExecutionTimeout: 1 * time.Minute,
5451
}
52+
r.Feature.StartWorkflowOptionsMutator(&opts)
5553
return r.Client.ExecuteWorkflow(ctx, opts, workflow, args...)
5654
}
5755
}

Diff for: harness/java/io/temporal/sdkfeatures/PreparedFeature.java

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ public class PreparedFeature {
66

77
static PreparedFeature[] ALL =
88
PreparedFeature.prepareFeatures(
9+
activity.basic_no_workflow_timeout.feature.Impl.class,
910
activity.retry_on_error.feature.Impl.class,
1011
activity.cancel_try_cancel.feature.Impl.class,
1112
child_workflow.result.feature.Impl.class,

0 commit comments

Comments
 (0)