Migrators panic rather than returning errors

[bgentry]

For example, I didn't specify a default retention period on the domain I was trying to register. When I called migrator.Migrate(), this exception got raised:

panic: com.amazon.coral.validate#ValidationException: 1 validation error detected: Value '' at 'workflowExecutionRetentionPeriodInDays' failed to satisfy constraint: Member must have length greater than or equal to 1 [recovered]
    panic: com.amazon.coral.validate#ValidationException: 1 validation error detected: Value '' at 'workflowExecutionRetentionPeriodInDays' failed to satisfy constraint: Member must have length greater than or equal to 1

goroutine 6 [running]:
testing.func·006()
    /usr/local/go/src/testing/testing.go:441 +0x181
github.com/sclasen/swf4go.(*DomainMigrator).register(0xc20802a040, 0x3e5970, 0x1f, 0x3cadb0, 0x13, 0x0, 0x0)
    /Users/bgentry/go/src/github.com/sclasen/swf4go/types.go:94 +0x96
github.com/sclasen/swf4go.(*DomainMigrator).Migrate(0xc20802a040)
    /Users/bgentry/go/src/github.com/sclasen/swf4go/types.go:71 +0x3dd
github.com/sclasen/swf4go.(*TypesMigrator).Migrate(0xc20801e480)
    /Users/bgentry/go/src/github.com/sclasen/swf4go/types.go:44 +0x142

It looks like there are a number of places in the codebase where errors aren't propagated up to the caller and panics are used instead. I think I also spotted some places where errors are swallowed altogether.

I'd be happy to take a crack at fixing some of these if you want.

[sclasen]

@bgentry So in the case of the migrators specifically, I was expecting the use to be in your main(), since you probably dont want to continue if there are errors...so run the migrator first thing and if it bombs, you dont want to keep running.

That said, if you think having errors returned is helpful, we can add a MustMigrate form, or some such, that panics with the returned error.

Feel free to take a crack.

One thing to note is that panics inside Decider calls are handled by the FSM machinery, this is mostly to let you avoid writing lots of boilerplate error handling around serialization/deserialization inside Deciders. If your state or event de/serialization isnt working you are probably broken, so just panic, and let the FSM transition you to an error state.

func Decide(fsm *FSMContext, h HistoryEvent, data interface{}) Outcome {
//prefer
    easySerialization := fsm.Serialize(data)  //panic and auto-transition to error state on error

    annoyingSerialization, err := fsm.Serializer().Serialize(data)
    if err != nil {
        // what to do other than go to an error state.
        // of course if you have serialzations or other things that can reasonably fail, by all means handle the error yourself
    }    
}

[bgentry]

Yeah, I agree that it's useful to have some auto-recovery for work loops. The net/http package does this with servers, for example (a panic from one request doesn't nuke the whole server).

It can be tough to decide where to draw the line. I did notice the Serialize()/Deserialize() case in particular, and my expectation was that I could decide how to handle such errors within my FSM. But I'm happy to wait until I've spent a little more time actually using the package before making up my mind on it :)

[sclasen]

yep, thats why there is also the Serializer() func on FSMContext, if you want to do the error handling its up to you.

the FSMContext Serialize, Deserialize, and EventData funcs are just attempting to limit boilerplate in Deciders, but you can not use them and do the error handling yourself for sure.

[bgentry]

Actually, somewhat of a tangent on the topic of migrators... How are you organizing what needs to be registered and deprecated for domains, workflows, and activities (along with the versions for each)? Some kind of large map that you iterate through for each of those?

[sclasen]

Pretty much just doing this kind of thing, though I havent got to doing too much deprecation yet. I also didnt spend a huge amount of time on ergonomics here, so open to better ideas if any occur to you.

func migrate() {
    client := swf.NewClientWithHttpClient(config.SWF.AwsAccessKeyId, config.SWF.AwsSecretAccessKey, swf.USEast1, instrument.SWFHttpClient())
    migrator := &swf.TypesMigrator{
        DomainMigrator:       domains(client),
        WorkflowTypeMigrator: workflows(client),
        ActivityTypeMigrator: activities(client),
        StreamMigrator:       streams(client),
    }
    migrator.Migrate()
}

func domains(client *swf.Client) *swf.DomainMigrator {
    return &swf.DomainMigrator{
        Client: client,
        RegisteredDomains: []swf.RegisterDomain{
            swf.RegisterDomain{
                Name: config.SWF.Domain,
                WorkflowExecutionRetentionPeriodInDays: config.SWF.WorkflowExecutionRetentionPeriodInDays,
            },
        },
    }
}

//same pattern for workflows,activities,streams