Catch SomeException and decorate with seed

[eborden]

Replay via seed is extremely useful for determining if a circumstance is flakiness or intrinsic. Non HUnit exceptions are not caught by graphula so unexpected IO exceptions cannot be replayed. Catching and decorating SomeException with the seed value can enable this workflow.

SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""}

[pbrisbin]

I've implemented decorating any exception e (other than HUnitFailure) as an AnnotatedException e with the seed as an annotation.

With this change as I have it, you'd now get:

-- option 1
uncaught exception: AnnotatedException
AnnotatedException {annotations = [Annotation @GraphulaSeed -6194587210816537995,Annotation @CallStack [("checkpoint",SrcLoc {srcLocPackage = "graphula-2.0.0.0-6RxeicdAygU6UOn5C6N1Qn", srcLocModule = "Graphula", srcLocFile = "src/Graphula.hs", srcLocStartLine = 275, srcLocStartCol = 34, srcLocEndLine = 275, srcLocEndCol = 44})]], exception = SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""}}

With a little more work, I could probably avoid the automatic @CallStack annotation, and get something a bit less noisy:

-- option 2
uncaught exception: AnnotatedException
AnnotatedException {annotations = [Annotation @GraphulaSeed -6194587210816537995], exception = SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""}}

With more work, we could implement something like AnnotatedException but with a cleaner Show:

-- option 3
uncaught exception: GraphulaWithSeed
GraphulaWithSeed {seed = -6194587210816537995, exception = SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""}}

-- or maybe
uncaught exception: GraphulaWithSeed
GraphulaWithSeed -6194587210816537995 (SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""})

This is as nice as we can get without violating the convention that a Show instance ought to be legal Haskell syntax and not something "Human friendly", and hspec uses show[^1]

Only by violating that convention could we hope to get things as nice as I imagine the original intent of this issue was:

-- option 4
uncaught exception: GraphulaWithSeed
Graphula with seed: -6194587210816537995
SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""}

So, I don't think we can annotate exceptions abstractly (e.g. as SomeException) without introducing these trade-offs. I think the choices are:

1. Simple thing, ugly output
2. Medium complex thing, medium-ugly output
3. Most complex thing, less-ugly output
4. Most complex thing, convention-violating implementation, least-ugly output
5. Do nothing, close as wontfix

Anyone have an opinion on what to do? /cc @chris-martin

[^1]: while hspec can be given a flag to use displayException, I don't think we should rely on that while evaluating these choices.

[chris-martin]

The simplest thing doesn't seem excessively offensive to me.

Maybe the display details ought to be up to context, e.g. in a spec hook. If instead of a Text annotation you used a type for this like

newtype GraphulaSeed = GraphulaSeed Int

uncaught exception: AnnotatedException
AnnotatedException {annotations = [Annotation @GraphulaSeed -6194587210816537995,Annotation @CallStack [("checkpoint",SrcLoc {srcLocPackage = "graphula-2.0.0.0-6RxeicdAygU6UOn5C6N1Qn", srcLocModule = "Graphula", srcLocFile = "src/Graphula.hs", srcLocStartLine = 275, srcLocStartCol = 34, srcLocEndLine = 275, srcLocEndCol = 44})]], exception = SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""}}

... then a spec could catch and display however it liked?

[pbrisbin]

I've updated my comment with the newtype tweak.

[pbrisbin]

I'm leaning towards Option 3, second variation.

uncaught exception: GraphulaWithSeed
GraphulaWithSeed -6194587210816537995 (SqlError {sqlState = "57014", sqlExecStatus = FatalError, sqlErrorMsg = "canceling statement due to statement timeout", sqlErrorDetail = "", sqlErrorHint = ""})

1. It's very low noise and easily readable, while not violating show conventions

2. It's almost equivalent syntax/complexity as option 1, without any new new dependency

   - checkpoint [seed] f
   + handle (throwIO . GraphulaWithSeed seed) f
   

[pbrisbin]

I guess there is an option 5, to actually turn any caught exception into an HUnitFailure that we pre-format however we like, e.g. to look like option 4.