haskell / core-libraries-committee

95 stars 16 forks source link

Exception backtrace proposal: Part 1: Annotations #200

Closed bgamari closed 11 months ago

bgamari commented 1 year ago

Tracking ticket: #164

This proposal attempts to summarise the interface design of the exception annotation scheme proposed in GHC Proposal #330. Specifically, this proposal covers the changes described in sections 2.1, 2.3, 2.4, and 2.5.

Note that the GHC Proposal is free-standing; no reading of the discussion which lead to its current accepted state should be necessary to understand its contents. Consequently, to avoid repetition I will refer back to the GHC Proposal instead of repeating myself here. I will, however, attempt to give some color to the interfaces by providing typical usage examples where necessary. However, the GHC Proposal is to be considered the canonical definition of the interfaces; in particular, section 2 and its subsections precisely captures the changes proposed in base.

Goal

The goal of this work is to allow users to more easily locate the causes of exceptions within their program. In particular, we note that when troubleshooting the context in which an exception occurred can be just as important in identifying the cause as the particulars of the event itself. For this reason, "cause" here may mean many things:

Since the particular information necessary to determine the "cause" of an exception can be quite domain-dependent, we propose a general-purpose annotation mechanism following the model of the annotated-exception library. This mechanism allows the user to attach dynamically-typed annotations to exceptions, which can be later inspected and displayed.

With this general-purpose mechanism in hand, we define a set of annotation types for capturing various backtrace flavours (namely, HasCallStack stacks, profiling cost-centre-stacks, DWARF execution stacks, and native Haskell execution stacks).

Exception annotations

The proposed annotation mechanism is introduced by extending the existing SomeException type, which represents the root of the exception hierarchy (readers wanting a refresher for GHC's exception system are referred to "An Extensible Dynamically-Typed Hierarchy of Exceptions"). This type is currently defined as:

-- Currently in Control.Exception
data SomeException = forall e. (Exception e) => SomeException e

We propose (section 2.4) to add an additional implicit parameter context to the SomeException data constructor, following the model of HasCallStack:

-- In Control.Exception
data SomeException = forall e. (Exception e, HasExceptionContext) => SomeException e

type HasExceptionContext = (?exceptionContext :: ExceptionContext)

someExceptionContext :: SomeException -> ExceptionContext

Since this addition would represent a breaking change, we propose (section 8.6) to provide defaulting logic in the typechecker, again following the model of HasTypeStack.

As described in section 2.3, ExceptionContext is an abstract, order-preserving collection of annotations. As a concrete representation we propose to use a simple List:

-- In Control.Exception.Context
newtype ExceptionContext = ExceptionContext [SomeExceptionAnnotation]
instance Monoid ExceptionContext
instance Semigroup ExceptionContext

Exception annotations are dynamically-typed values distinguished by the ExceptionAnnotation typeclass (section 2.1):

-- In Control.Exception.Annotation
data SomeExceptionAnnotation where
    SomeExceptionAnnotation ::
      forall a. (ExceptionAnnotation a) => a -> SomeExceptionAnnotation

Where the ExceptionAnnotation class provides for rendering of annotations to a user-facing string (following the model of displayException; section 2.1):

-- In Control.Exception.Annotation
class (Typeable a) => ExceptionAnnotation a where
  displayExceptionAnnotation :: a -> String

  default displayExceptionAnnotation :: Show a => a -> String
  displayExceptionAnnotation = show

We provide a few combinators for manipulating exception context (section 2.3):

-- In Control.Exception.Context
emptyExceptionContext :: ExceptionContext
addExceptionAnnotation :: ExceptionAnnotation a => a -> ExceptionContext -> ExceptionContext
getExceptionAnnotations :: ExceptionAnnotation a => ExceptionContext -> [a]
getAllExceptionAnnotations :: ExceptionContext -> [SomeExceptionAnnotation]

Adding annotations to exceptions

Attaching annotations to an exception may be accomplished using an introduced addExceptionContext function:

-- In Control.Exception
addExceptionContext :: (ExceptionAnnotation a)
                    => a -> SomeException -> SomeException

It is anticipated that users may frequently want to run an IO action, ensuring that any exceptions thrown therein are given an annotation. This can be accomplished using the annotateIO function (section 2.4):

-- In Control.Exception
annotateIO :: ExceptionAnnotation a => a -> IO r -> IO r

Showing annotations

Naturally displayExceptionAnnotation can be used to display each of the annotations contained within an ExceptionContext (section 2.3):

-- In Control.Exception.Context
displayExceptionContext :: ExceptionContext -> String
displayExceptionContext (ExceptionContext anns0) = go anns0
  where
    go (SomeExceptionAnnotation ann : anns) = displayExceptionAnnotation ann ++ "\n" ++ go anns
    go [] = "\n"

To make annotations visible to users, we propose to:

  1. Use displayException in GHC's top-level exception handler (CLC#198)
  2. Teach the displayException implementation of SomeException to display attached ExceptionContext via displayExceptionContext (section 2.10)

Providing exception context to handlers

Typically when users use handle or catch they do so against a concrete exception type, not SomeException. Since this pattern would prevent access to ExceptionContext, we propose adding a convenient wrapper to expose an arbitrary exception with its context. The Exception instance of this wrapper allows it to be used in both throwing and catching contexts:

-- In Control.Exception
data ExceptionWithContext a = ExceptionWithContext ExceptionContext a

instance Show a => Show (ExceptionWithContext a)

instance Exception a => Exception (ExceptionWithContext a) where
    toException (ExceptionWithContext ctxt e) = SomeException e
      where ?exceptionContext = ctxt
    fromException se = do
        e <- fromException se
        return (ExceptionWithContext (someExceptionContext se) e)
    displayException = displayException . toException

Usage

In the case of a server application, the author may wish to augment exceptions thrown within a request handler with information about the request being handled. This can be achieved with the above interfaces as follows:

data Request
instance ExceptionAnnotation Request where
    displayExceptionAnnotation req =
        "While handling request " ++ show req

handler :: Request -> IO ()
handler req = annotateIO req $ do ...

This example is representative of what we believe will be typically needed by end-users.

Far fewer users (e.g. structured and application-specific logging infrastructure) will need to directly inspect exceptions' context.

Migration

Thanks to the default behavior described in Section 8.6, the HasExceptionContext constraints introduced in SomeException should be discharged automatically.

All other changes are strict additions which should require no migration effort on the part of library authors or application developers.

Bodigrim commented 1 year ago

Dear CLC members, please review the subproposal here. The general approach to the problem has been approved by GHC SC, it would be unproductive to discuss it anew; but we can and should focus on library-level details.

There will be no MR for this subproposal alone, the exact implementation would await for us approving or amending all related subproposals.

If there is no substantial discussion, I'll ask for a vote in 7 days from now.

CC @mixphix @hasufell @tomjaguarpaw @velveteer @parsonsmatt @angerman

parsonsmatt commented 1 year ago

Far fewer users (e.g. structured and applications-specific logging infrastructure) will need to directly inspect exceptions' context.

I think I disagrree with this - most of our uses of these annotations requires inspecting the content. In our work app, we've got:

data WrapMercuryAnnotation where
  WrapMercuryAnnotation :: (MercuryAnnotation a) => a -> WrapMercuryAnnotation

class MercuryAnnotation a where
  modifyBugsnagReport :: a -> BugsnagEvent -> BugsnagEvent

And then we can tryAnnotations :: [Annotation] -> ([WrapMercuryAnnotation], [Annotation]) to provide a much richer interface. This has a multitude of uses, including directing Bugsnag reports to specific Slack channels.

I don't think this proposal is terribly relevant, since hte "subclass" approach works pretty well here.

EDIT:

Actually, if we're going to use annotateIO :: (ExceptionAnnotation a) => a -> IO r -> IO r then we ought to include toAnnotation and fromAnnotation methods on the class. Otherwise it'll be much more challenging to incorporate the annotation subclasses.

tomjaguarpaw commented 1 year ago

The implementation of SomeException is exposed by base so the change from

data SomeException = forall e. (Exception e) => SomeException e

to

data SomeException = forall e. (Exception e, HasExceptionContext) => SomeException e

requires CLC review. However, the rest of it need not impinge on base, need it? Couldn't it all go in ghc-experimental and avoid the need for CLC involvement? Later, once the design has been proved out in practice, the CLC could consider it for inclusion in base.

[EDIT: correctedghc-internals to ghc-experimental]

Bodigrim commented 1 year ago

@tomjaguarpaw this would mean pretty much every production application depending on ghc-internals, which defeats its purpose.

I mean, using ghc-internals for bleeding-edge dependently-typed stuff is desirable, but exception backtraces are very much practice-oriented feature, and pushing industrial users to use ghc-internals is a one-way road.

tomjaguarpaw commented 1 year ago

this would mean pretty much every production application depending on ghc-internals, which defeats its purpose

Not necessarily. Someone (even GHC HQ) could (in fact should) produce a wrapper library around the new feature whose interface they strive to keep stable. (In fact, that's exactly how I expect ghc-internals to be used in most cases.) I don't see why this feature desperately belongs in base yet.

[EDIT: I meant ghc-experimental not ghc-internals. Maybe that was the cause of the objection?]

bgamari commented 1 year ago

Couldn't it all go in ghc-internals and avoid the need for CLC involvement? Later, once the design has been proved out in practice, the CLC could consider it for inclusion in base.

I don't see how this would work. SomeException is central to the design of the extensible exceptions machinery and is already necessarily exposed from base. The only way we could avoid exposing this change via base would be to instead replace the data constructor with a pattern synonym, an option which we already considered and rejected in the GHC proposal.

tomjaguarpaw commented 1 year ago

I'm confused. I explicitly said that the SomeException change must go in base (but I don't see anything else that necessarily needs to) so at least one of us is misunderstanding something here. It may be me!

Am I wrong that the rest of the proposal need not be exposed through base?

bgamari commented 1 year ago

Ahh, I see; indeed I had misunderstood your comment. However, for the same reasons noted by @Bodigrim I would strongly oppose including only in ghc-internals. We could, of course, place the new interfaces in ghc-experimental; I am rather ambivalent towards doing so.

tomjaguarpaw commented 1 year ago

Oh sorry, I did mean ghc-experimental. Naturally, it's not internal!

angerman commented 1 year ago

Is this expected to be subject to rapid change? That is are the authors of this proposal expecting the interface design to change? Do they want to reserve the right to do so without notice and proper change evolution?

If the answer is yes, then this ought to be in ghc-experimental. I would also argue strongly that any such feature subsequently should only be talked about with a strong "experimental: subject to change" notice.

If the answer is no, then base seems the better choice?

angerman commented 1 year ago

There is one problem I see though, this is solving a real world problem for many I believe. And we want to prevent people at the same time from depending on ghc-experimental.

Do we risk people depending on ghc-experimental out of necessity here? Would that defeat the whole purpose of the split?

tomjaguarpaw commented 1 year ago

Is this expected to be subject to rapid change?

I suppose if the authors can say for certain that it won't be then base is the correct location. But why pay the cost of having to determine that up front?

Maybe in this case we can say that we expect the design to be stable. In that case I don't have a strong objection to incorporating this in base. But in general I think a sensible policy would be for GHC to incorporate innovative new features quickly into ghc-experimental, get a certain degree of real-world feedback, and only then propose a stable API for inclusion into base.

angerman commented 1 year ago

@tomjaguarpaw I agree with

But in general I think a sensible policy would be for GHC to incorporate innovative new features quickly into ghc-experimental, get a certain degree of real-world feedback, and only then propose a stable API for inclusion into base.

We need to be very explicit that stuff in ghc-experimental is clearly marked as preview/experimental features. If we end up in a situation where everyone just cargo-cults ghc-experimental (or a hypothetical -experimental flag), those have no value.

This is a preview features that can be experimented with using ghc-experimental. It is highly discouraged to use this feature in production or packages uploaded to hackage. The API is expected to stablalize in GHC X.Y.Z and move into base?

If everyone just ends up depending on ghc-experimental, what value did the base-split have?

tomjaguarpaw commented 1 year ago

If everyone just ends up depending on ghc-experimental, what value did the base-split have?

That is a fair question, but the converse is also a fair question: if we keep putting unstable stuff in base what value did the base split have?

If the proposers are adamant that their design has taken into account all possibilities and they have ruled out the need for changes (say in the next three base versions) then by all means let's include it into base. Otherwise I think the right thing to do is to expose it through a new package (that depends on ghc-experimental, so consumers don't depend on ghc-experimental directly).

parsonsmatt commented 1 year ago

I don't see why this feature desperately belongs in base yet.

Well, because I desperately want it, for one 😄

I wrote annotated-exception to solve the pain points we have with Haskell exceptions. Thanks to this library, our work app has reliable (though partial) callstacks, along with custom annotations that allow us to modify the Bugsnag representation of our errors. This means that we can know which organization/user experienced an exception, and on what specific workload. We have extremely good understanding of the runtime situation that caused an exception. I really can't sell it enough. It's a game changer for understanding and diagnosing Haskell applications at scale, and takes the experience from "worse than Ruby/Java/Go" to "way better than anything I've ever done before."

However, to get it working, you have to use a library-provided catch that can see through the AnnotatedException wrapper - what is actually thrown is an AnnotatedException E, and if you use Control.Exception.catch to try and handle an E, it won't work. As a library author, I cannot use annotated-exception without forcing my users to also use it. As an application developer, I must consciously unwrap AnnotatedExceptions before I pass my code off to a library, which may call the function expecting to catch a specific exception type, and be unable to do so.

Some of this is mitigated with the exceptions package and using MonadThrow, MonadCatch, etc. Since I can say instance MonadCatch App where catch = Control.Exception.Annotated.catch, and a library function can use (MonadCatch m) => m _ to indicate that it will catch exceptions, I'm fine if I instantiate m ~ App. However, this does nothing if I'm using instance MonadCatch IO, or any other MonadCatch instance.

So there's an exceedingly good tool, but with some sharp edges and important caveats. I can mitigate some of these by providing integration for libraries (ie yesod could integrate annotated-exception and support it directly). However, the only real way to get this functionality to everyone in Haskell is by implementing it directly in SomeException.

Maybe in this case we can say that we expect the design to be stable.

So, to play devil's advocate in my own post,

I don't know how much I expect the API to be stable. It's a brand new feature. The design in question has not actually been used anywhere. The design does differ in some important ways from the API that I have in annotated-exception that is used in production.

The API change is relatively small. As you mentioned, it's just an extra constraint on SomeException, which GHC can automagically solve, much like HasCallStack is automagically solved. The rest of the API change could be in ghc-experimental and then possibly exposed through a variant of annotated-exception as a package. End users would continue using throw, catch, and SomeException without visible difference, and depending on ghc-experimental would allow you to also have annotateIO and inspect the annotations. This approach still gets almost all of the benefits: every throwIO will have a CallStack. Every library that uses catch won't have to know or care about annotated-exception, it'll Just Work. Users that want to opt-in to the new API can do so. I can make annotated-exception way safer and easier to use.

The other question is about the behavior change of catch. I think for most users, there won't be a difference. However, the extra code for re-attaching the exception context in rethrown exceptions will add some overhead. If you're catching in a tight loop, this may cause performange degradation, and we may want to provide a catchNoAnnotations or whatever that doesn't do that.

angerman commented 1 year ago

I think it's important to get clarity as to what

expect the design to be stable

implies. To me this implies there can not be backwards incompatible changes to the API without change evolution/deprecation/migration phase. Downstream consumers of this feature must not be broken from one release to the next. Existing code must still compile for at least one or more ghc releases.

This explicitly does not mean things can't change, but they can not change in backwards incompatible ways without appropriate deprecation/migration period.

I to consider this feature rather high value and most needed. And as such I'd like to see this in base rather than ghc-experimental to not have ghc-experimental leak into dependency trees out of necessity. This however to me comes with the obligation that the proposed interface will not change without a proper change evolution. We may have two interface one new one, and a deprecated one for a while or some other form of migration; but existing consumers must not break from one release to the next.

tomjaguarpaw commented 1 year ago

I don't see why this feature desperately belongs in base yet.

Well, because I desperately want it, for one 😄

I want you (and everyone) to have it too. The question is, where is the appropriate place for it to be placed?

I think it's important to get clarity as to what

expect the design to be stable

implies. To me this implies there can not be backwards incompatible changes to the API without change evolution/deprecation/migration phase

I'm basically in agreement. Personally, I don't want the people designing this interesting new piece of functionality to have its evolution held back by the need to adhere to deprecation cycles tied to base releases. That imposes a huge drag! That's why I think exposing it from ghc-experimental might be a good place to start. But if the proposers really want to tie their own hands by exposing it from base then I'm not going to stand in their way.

angerman commented 1 year ago

@tomjaguarpaw I think that's where we disagree. If we end up putting everything into ghc-experimental, because "base" (adherence to some form of backwards compatibility) is too much work, we will end up with everyone just depending on ghc-experimental. That certainly can't be the goal, as it means ghc-experimental is the new base.

tomjaguarpaw commented 1 year ago

But what exactly do we disagree on? We surely agree that some things should go straight into base and some things should start life ghc-experimental, right? So all that I think we disagree on is whether this particular thing falls into the former group or the latter.

I'm not actually particularly opposed to putting it straight into base anyway. It looks like a well-designed and valuable proposal. But I think it's worth noting that if we'd had ghc-experimental when the GHC committee accepted the corresponding GHC proposal (six months ago) then we could have had this functionality in 9.8 and could be trying it out already! I think that's the whole point of ghc-experimental, to release new stuff earlier. And we shouldn't make it a big deal to take things that start life in ghc-experimental and stabilise them quickly into base after one or two GHC releases. There's very little cost to doing that.

Ericson2314 commented 1 year ago

Yeah I think new things should rarely go directly in base, because is a recipe for not taking the stability problem seriously. On the flip side, stuff should not stay in ghc-experimental very long either, for the reasons @angerman mentioned. This is just navigating between Scylla and Charybdis.

angerman commented 1 year ago

@tomjaguarpaw ok let me try to out it this way, maybe that makes my view clearer.

  1. Highly popular/sought after feature makes it into ghc-experimental
  2. Everyone depends on ghc-experimental, likely transitively due to hackage dependencies.
  3. Feature breaks code without warning, after all it was in ghc-experimental.
  4. Everyone who had ghc-experimental in their transitive dependency closure breaks as well (or is forced to upgrade and integrate new major versions).

At this point we are back to square one. If it's in base or ghc-experimental makes virtually no difference everyone depends on ghc-experimental now anyway (potentially just transitively).

The longer features stay in ghc-experimental, the more likely it is ghc-experimental becomes just another dependency and we added a lot of overhead for nothing.

If we expect there to be high demand for a high profile feature, we might want to argue for it being in base from the start, which also means it can't break easily, to prevent the above scenario and the separation from being eroded.

Or we need to put strict limits on features in experimental. They can be there for at most two releases before they have to move to base or be removed?

Ericson2314 commented 1 year ago

I am also talking to @angerman about this, but I don't think that is right back to where we started. Today, we can't even agree on what things are and are not stable, nor have any simple and reliable way to audit whether one is staying within stable.

If tomorrow everyone is knowingly using unstable things (because static analysis tells them they are, they are forced to pass a flag consenting to doing so), then the question is "why is Stable Haskell not good enough?" That is a much more orderly and productive conversation!!

Or we need to put strict limits on features in experimental. They can be there for at most two releases before they have to move to base or be removed?

Yeah I think that is fine.

tomjaguarpaw commented 1 year ago

I think I'm still missing something that's clear to you but not me. Maybe I should explain what I am envisaging:

  1. CLC approves adding the HasExceptionContext constraint. Not too controversial.
  2. Everything else goes into a new package called exception-backtraces, made by someone at GHC HQ.
  3. exception-backtraces may get its functionality from ghc-experimental, or ghc-internals, or it may be implemented directly in that package, it doesn't really matter, because the important thing is that ...
  4. no consumer of the new functionality needs to depend on ghc-experimental. They depend on exception-backtraces. Consumers may depend transitively on ghc-experimental, but that's an implementation detail that is never relevant to them.
  5. The feature is really useful, so a huge proportion of Hackage comes to depend on exception-backtraces (directly or transitively)
  6. If a later GHC makes breaking changes to unrelated parts of ghc-experimental or ghc-internals that is no problem at all, because exception-backtraces did not depend on those parts. Consumers of exception-backtraces do not even need to bump bounds!
  7. If, after one or two GHC versions, experience proves that the design was correct then we incorporate it into base. This is completely non-breaking to existing consumers of exception-backtraces (and we can even have exception-backtraces re-export its functionality directly from base and live around indefinitely as a compatibility shim)
  8. If, however, experience proves that we need to change or even break the API then the maintainers of exception-backtraces can do so without getting CLC involved

So how does this approach measure up against others? We have two alternatives:

A. Incorporate this proposal into base now B. Incorporate this proposal into exception-backtraces now and stabilise it into base as soon as it has proved itself in practice

Let's consider possibilities under the two alternatives:


So I don't think option B has big drawbacks compared to option A, and I think it has a lot to recommend it. To reiterate, I'm not opposed to option A, I'd just like is to think what's the most appropriate approach here.

B has the benefit of freeing up a lot of CLC time and energy by deferring up front design review to the reality of practice. Frankly, for a design as well fleshed out as this one, I'm not sure I can add much in an up front review and I think getting it in the hands of users ASAP is the right thing to do. B has the downside that if this design really is stable, we force everyone to use exception-backtraces for one or two releases rather than base. I see that as annoying but not a big deal.

So, @angerman, could you let me know if that helps clarify what I am envisaging and whether you still think B has significant downsides? If you do think that, could you elaborate on what they are, because I'm still not seeing them.

angerman commented 1 year ago

@tomjaguarpaw so you argue that exception-backtrace should provide the stable interface. Will exception-backtrace then be developed in tandem with ghc-experimental?

How do I know that, even though I depend on ghc-experimental only transitively, that my code will not break from one ghc version to the next.

The goal of stability at least to me is that I can trivially swap out the compiler, without needing to make any material code changes.

cabal -w <new compiler> build all

Should just work. That's where I want to get.

tomjaguarpaw commented 1 year ago

The goal of stability at least to me is that I can trivially swap out the compiler, without needing to make any material code changes.

cabal -w <new compiler> build all

Should just work. That's where I want to get.

Great. I want to get that too. Can you explain why putting this functionality in base makes it more likely than putting it in exception-backtraces?

tomjaguarpaw commented 1 year ago

Actually, let me short-circuit that question and just propose the reason that I think you're getting at:


Putting this functionality in base is better for stability than putting it in exception-backtraces because if it is put in base then CLC can assert its right to forbid future breaking changes to it. If it is put in a new package, exception-backtraces, then CLC would not have any authority to forbid breaking changes, so breaking changes may well happen intentionally or accidentally.

Therefore, in the interests of getting cabal -w <new compiler> build all to work without material code changes we should put this functionality in base.


Is that a fair summary of your position?

angerman commented 1 year ago

@tomjaguarpaw no, I'm not looking to put it into base to have a say in this. My concern is primarily that ghc-experimental is going to become base; and thus the meaning of base and ghc-experimental eroding over time.

To me ghc-experimental should be independent of base, and due to stability concerns, I can not depend on ghc-experimental in production.

If the design is truly up in the air, and we expect this to break rapidly, then I shouldn't be using it in production.

I've had some lengthy debate with @Ericson2314 and came to the realization, what I'm advocating for is probably still far, far out.

parsonsmatt commented 1 year ago

It's probably reasonable for users of ghc-prim and ghc-experimental &c to expect to use CPP to provide compatibility across GHC versions. This pushes the responsibility towards the library authors that are on the bleeding edge. I don't know of a good way of enforcing that, aside from a bot that builds versions of packages that depend directly on ghc-experimental with different GHC versions and whines loudly if it doesn't work.

angerman commented 1 year ago

@parsonsmatt true. I think a stackage or similar automation is essential to arrive there. We are still early in all this and will need to see how it pans out. I do hope that some companies will start using -no-experimental flags if they are given an easy option to do so, and the promise is that their code will still compile if they swap the compiler. And this would hopefully produce collective interest a good non-experimental subset of packages and dependencies.

tomjaguarpaw commented 1 year ago

@angerman As far as I can tell we have the same goal but diverging ideas about how to achieve the goal. I still don't understand why or how they diverge, but that's OK, it's not something we need to resolve now. This was an interesting discussion and ultimately I'm still in support of incorporating this proposal directly into base.

I think it would be useful to have @bgamari estimate the likelihood of the need for breaking changes to this API in the near future.

bgamari commented 1 year ago

Thank you all for the useful discussion and apologies for the latency; I have been travelling this week which has made it somewhat difficult to stay on top of this.

If the proposers are adamant that their design has taken into account all possibilities and they have ruled out the need for changes (say in the next three base versions) then by all means let's include it into base.

I naturally cannot claim to have taken into account all possibilities however I do think that the design as it sits is fairly unlikely to require changes in the near future. Afterall, over its three-year lifetime the proposal has gone through three full rewrites and numerous revisions. Moreover, the majority of its surface area is a fairly generic mechanism, the design of which is fairly straightforward. I would expect any changes that we do want to make can be fairly easily done with a suitable deprecation cycle.

While I will happily implement follow whatever decision the CLC makes here, I do think we should take care to weigh the cost of such stability measures against the likelihood and impact of changes necessary in the proposed interfaces. Afterall, asking users to add dependencies is not free, nor is maintaining compatibility shim well into the future. My personal sense is that the likelihood of future breaking changes is small enough that such measures are not justified.

angerman commented 1 year ago

I would expect any changes that we do want to make can be fairly easily done with a suitable deprecation cycle.

This reads to me like the authors expect the interface to be stable and commit to a change evolution policy that won't see breaking changes without notice period.

This to me is the criteria that fulfills putting it into base and not ghc-experimental. After all the authors do not consider it experimental.

Bodigrim commented 1 year ago

I’m strongly in favor of putting it into base, largely aligned with @angerman‘s reasoning above.

hasufell commented 1 year ago

I think this is useful... I'm unable however to clearly see all the intricacies, since I've not used anything similar in a production codebase like Matt has. Given that it's not technically a breaking change according to 8.6, I think this is ok to go into base, unless major objections to the design are raised.

Reading through the alternatives it seems this approach is the least invasive and most backwards compatible one. And reading @parsonsmatt's remarks on annotated-exceptions it seems to me it's also a good minimal design that allows richer interfaces to be built on top.

I think we will really only see all the problems with the design once people try to actually use it: end users have great creativity to break expectations and use your code in unforeseen ways.

In that sense... I'm not entirely sure whether all of it should go into base at once. From what I understand, we could:

  1. merge the SomeException change
  2. add everything else to ghc-experimental
  3. observe how end users use it
  4. either merge to base after an experimentation period (say, a year) or drop everything and revert the SomeException change (which would not break anything afaiu... anything that can utilize HasExceptionContext would be in ghc-experimental and dropping the implicit parameter shouldn't break anything?)

But I'm not strongly arguing for this.

hasufell commented 1 year ago

Wrt the ghc-experimental discussion:

My concern is primarily that ghc-experimental is going to become base; and thus the meaning of base and ghc-experimental eroding over time.

ghc-experimental becomes meaningless if we don't use it. We're not responsible for what the community does with it and we can't control it. But we can communicate the expectations. At least for things where CLC is involved, we can set an experimentation phase (e.g. 6 months) and make a final conclusion afterwards: merge to base or delete the feature.

bgamari commented 1 year ago

Hi all, is there anything preventing this from going to a vote?

mixphix commented 1 year ago

I think the value in providing more detailed debugging information from base must be weighted by its ease of use. This will only improve Haskell's runtime debugging capabilities, available to any user of ghci. Providing the functionality here, even though it may be subject to improvement, will prevent first-time Haskellers from being forced to configure a Cabal project before having access to better error messages!

hasufell commented 1 year ago

Hi all, is there anything preventing this from going to a vote?

Do you have an opinion on my proposed course of action in https://github.com/haskell/core-libraries-committee/issues/200#issuecomment-1773797532

Or would you rather see everything go into base at once?

Bodigrim commented 1 year ago

ghc-experimental becomes meaningless if we don't use it. We're not responsible for what the community does with it and we can't control it. But we can communicate the expectations. At least for things where CLC is involved, we can set an experimentation phase (e.g. 6 months) and make a final conclusion afterwards: merge to base or delete the feature.

My view is that the question is: would a reasonable enterprise user be tempted to depend on ghc-experimental asap just to lay hands on the feature? If the answer is yes (as it seems to be for exception backtraces) then we better put it in base directly.

It's not like a short experimentation phase makes much sense: the timespan between one GHC release and another feature freeze is too short to obtain any results. Realistically it would take 2-3 GHC releases before a new feature hits Stackage LTS and we get a meaningful amount of feedback. The problem is that at that point industrial users would not be looking to wait another year or two before the feature is in base officially and will just use ghc-experimental.

(The situation is different for, say, Dependent Haskell features, where the main feedback channel is through people living on the bleeding edge and happy to update and experiment in a tight loop)

I do feel that we are trying to avoid responsibility by pushing things into ghc-experimental. Given that the exception backtrace proposal was in making for ~3 years altogether, I imagine the design to be sufficiently mature. The proposer says:

I do think that the design as it sits is fairly unlikely to require changes in the near future.

parsonsmatt commented 12 months ago

Folks that don't want ghc-experimental can depend on annotated-exception today and get most of the benefits. annotated-exception could even have a flag for conditional dependency on ghc-experimental and underlying features, so that end users could be experimental-agnostic.

hasufell commented 12 months ago

I do feel that we are trying to avoid responsibility by pushing things into ghc-experimental. Given that the exception backtrace proposal was in making for ~3 years altogether, I imagine the design to be sufficiently mature.

I don't have a very strong opinion on this, but I'm not sure I share your confidence that the design won't change ad-hoc in 6 months time and we'll be seeing another set of proposals.

Generally, ghc-experimental is ok for industrial users to use too. It follows PVP and is clear about the guarantees. I'd expect industrial users to be much more aware of the expectations than others.

If there was a rising number of such industrial users and we had a way to know, that would also be a good case to say: it's probably time to move it to base.

My stance is: we should probably start using ghc-experimental as a procedure, rather than in exceptional cases.

bgamari commented 12 months ago

Do you have an opinion on my proposed course of action in #200 (comment)

While I do believe that the current design is unlikely to require breaking changes, I have no objection to adding the majority of the surface area to ghc-experimental first and stabilizing to base later.

To take up @Bodigrim's question,

would a reasonable enterprise user be tempted to depend on ghc-experimental asap just to lay hands on the feature

I suspect that the answer may be "yes" in some cases. However, I don't necessarily view this as a problem. Afterall, things exist in ghc-experimental to be used.

hasufell commented 12 months ago

+1 from me and I'd recommend to go through ghc-experimental, but am not demanding it.

bgamari commented 11 months ago

@Bodigrim, any further thoughts on this?

Bodigrim commented 11 months ago

Thanks for patience, @bgamari!

Re-reading the proposal and the discussion, I remain at loss how one imagines a split between base and ghc-experimental. If we change

-data SomeException = forall e. (Exception e) => SomeException e
+data SomeException = forall e. (Exception e, HasExceptionContext) => SomeException e

then HasExceptionContext must be defined in base, which implies that ExceptionContext, SomeExceptionAnnotation and ExceptionAnnotation and their instances all also go in base. Which really leaves us with

emptyExceptionContext :: ExceptionContext
addExceptionAnnotation :: ExceptionAnnotation a => a -> ExceptionContext -> ExceptionContext
getExceptionAnnotations :: ExceptionAnnotation a => ExceptionContext -> [a]
getAllExceptionAnnotations :: ExceptionContext -> [SomeExceptionAnnotation]
addExceptionContext :: ExceptionAnnotation a => a -> SomeException -> SomeException
annotateIO :: ExceptionAnnotation a => a -> IO r -> IO r
displayExceptionContext :: ExceptionContext -> String

Technically these functions could probably be put into ghc-experimental, but I don't see who is to benefit from such artificial butchering. These are extremely barebone combinators, which are very unlikely to change and are required even for most basic usage.

Since no one clearly articulated a cohesive way to split the proposal into more fine-grained parts (beyond "only HasExceptionContext goes into base", which is technically impossible, since it depends on other parts of the proposal), let me trigger a vote as is.


Dear CLC members, let's vote on the Part 1 of the exception backtrace proposal, enhancing SomeException with HasExceptionContext and adding basic tools to work with exception contexts as detailed above.

@tomjaguarpaw @hasufell @parsonsmatt @angerman @velveteer @mixphix


+1 from me, great stuff which we were longing for years.

hasufell commented 11 months ago

+1

tomjaguarpaw commented 11 months ago

+1

velveteer commented 11 months ago

+1

Bodigrim commented 11 months ago

@mixphix @angerman @parsonsmatt just a gentle reminder to vote.

mixphix commented 11 months ago

+1