Polymorphic envelope

[BrechtSerckx]

This PR solves #5 by generalizing the envelope used, as in this comment. I chose for this approach as it is more general than a phantom type parameter, and allows for using custom envelopes.

In the first commit, primed variants of existing combinators are defined, accepting an envel parameter of kind [*] -> * -> *. The existing combinators are redefined as aliases, providing envel = Envelope.

In the second commit, a FlatEnvelope type is defined as a wrapper around Envelope with a flat JSON representation.

Tests pass, and it does the job in my pet project. I don't use docs though.

Feedback is certainly welcome!

[cdepillabout]

@BrechtSerckx Thanks for working on this.

Could you add a small example of using this here: https://github.com/cdepillabout/servant-checked-exceptions/tree/master/servant-checked-exceptions/example

[BrechtSerckx]

Could you add a small example of using this here: https://github.com/cdepillabout/servant-checked-exceptions/tree/master/servant-checked-exceptions/example

Sure! I'll look into it.

[BrechtSerckx]

@cdepillabout I added documentation generation for FlatEnvelope and an example for using custom envelopes.

[cdepillabout]

Thanks for adding an example. I've been busy lately, but I'll try to get around to doing a good review of this sometime soon.

[cdepillabout]

@BrechtSerckx While I think the approach in this PR is more general, I'm wondering if the approach described in #5 (comment) would be easier to implement and maintain.

Almost all of the current code would basically be the same, but Envelope would gain an additional parameter.

Are there things that you want to do that wouldn't be enabled by this approach?

[cdepillabout]

Not being able to directly use pureErrEnvelope and pureSuccEnvelope when using a different envelope type is pretty inconvenient.

I guess all of the functions exported by Servant.Checked.Exceptions.Envelope would have to be wrapped?

[BrechtSerckx]

Not strictly necessary. My current implementation uses it like this: I program using normal Envelopes (EnvelopeT), and only right before returning the return value in the endpoint handler, I wrap it in a FlatEnvelope. Combined with an existing helper function, this makes the conversion of my server-side code only a oneliner.

[BrechtSerckx]

@BrechtSerckx While I think the approach in this PR is more general, I'm wondering if the approach described in #5 (comment) would be easier to implement and maintain.

Almost all of the current code would basically be the same, but Envelope would gain an additional parameter.

Both EnvelopeT and Envelope would need an additional parameter right? Also, you'd need parametrized servant combinators too, to give the serialization type.

I'll see if I can compare pros/cons:

envelope parameter:
Pro:

• general, can be used with custom open unions
• servant instances don't change much, s/(Throws|Throwing) Envelope/\1' envelope/g

Contra:

• a kinded parameter in the servant combinators can be confusing to newcomers?
• adds 2 typeclasses: one to get HTTP status from envelope (for servant-client), one to create envelopes (for servant-docs), to be implemented by envelopes
• when using a wrapper (as FlatEnvelope) one needs to wrap/unwrap once in each handler/client

Envelope with phantom type:
Pro:

• servant instances change even less, s/Envelope/Envelope' phantom/g, implementation stays the same

Contra:

• Envelope gets more complex
• all Envelope and EnvelopeT signatures need an additional parameter

Personally, I don't think having a an envelope parameter is a maintenance burden, as the code changes are quite small. A big part was a literal find/replace.

Are there things that you want to do that wouldn't be enabled by this approach?

At the moment, no.

[cdepillabout]

@BrechtSerckx Thanks for the summary.

I think the approach implemented in this PR (adding an envelope parameter to the servant combinators) is the "better" approach, and ideally we'd find a nice way to get this to work.

However, making users have to create wrappers around pureErrEnvelope and pureSuccEnvelope is a pain. Can you think of a way to prevent this?

Off the top of my head, I guess we could introduce a type-class for figuring out the correct Envelope type to use (which will be looking at the return value of servant handler), but adding an additional typeclass will just add complexity to an already complex library.

I was thinking that just adding an additional phantom type to Envelope would reduce the burden on the end-user. If they wanted a different JSON instance, all they would have to do is create a type alias to Envelope', and write the ToJSON instance they want.