error-stack: possible serde implementation for Report

[indietyp]

Pull Request: #792 #1290

---

This is the discussion related to the serde implementation of error-stack

[indietyp]

My idea would be twofold: from_serde() for a Frame (which holds an erased_serde::Serialize) and a serde serialize implementation.

The format I had in mind is:

{
   "messages": ["Printable A", "Printable B", "Root Error"],
   "attachments": ["serde attachment 1", "serde attachment 2", "serde attachment 3"],
   "sources": [
        ...
   ],
   "backtrace": [
       ...
   ],
   "spantrace": [
      ...
   ]
}

This implementation would group frames to limit the indentation and level until a group (frame has multiple sources) is encountered.

Backtrace and Spantrace would ideally be structured, but you are currently unable to extract the BacktraceFrame symbols and co from std::backtrace::Backtrace (but can do so from backtrace::Backtrace). This means I have low hopes that we can do so, which would ideally look like: {line: X, column: X, file: X, name: X}

attachments would be serde serializable frames (from the new Frame type).

[indietyp]

@TimDiekmann what do you think?

[TimDiekmann]

It won't be possible to reconstruct a Report from only JSON as all type of information is lost. Let's not go wild and provide a simple and straightforward Serialize implementation for Frame (and Report).

I currently see two ways to solve it.
(a) Be dumb, make it recursive (with deeper nesting).
(b) Traverse the sources as occurred with the .frames() iterator. These would require storing the sources as indexes.

I think (a) is more approachable because it's easy to implement, to understand, and deeply nesting is probably not super common. Additionally, (b) is tricky to get the indices for the sources and it's hard to reconstruct the error later on.
We cannot fulfill every user's needs. If a user needs a serialized struct in a special way, they would implement it themselves anyway using a newtype.

[indietyp]

I agree, (a) is definitely easier.

Isn't my initial proposal basically (a) and (b) combined?

We would still get nesting (maybe on Context and multiple groups), but limit the nesting and still get the necessary information out of it.

[TimDiekmann]

Yes, it feels like a hybrid, but IMO that doesn't really makes this easier.

[indietyp]

That's true. Arguably it makes things a bit harder, but(!) I am already using a very similar technique for the new Display implementation, so I am pretty confident that it is doable (it's actually pretty straight forward)

[indietyp]

Example:

        A
        |
        B
        |
        C
       / \
      E   F
      |
      G
      | 
      H

where:

A: Printable (message: "Printable A")
B: Serde (message: "<serde>")
C: Context (message: "Context C")
E: Opaque (backtrace)
F: Context (message: "Context F")
G: Opaque (spantrace)
H: Context (message: "Context F")

(all the names are still tbd)

@TimDiekmann a)
dubbed: simple

{
  "type": "message",
  "content": "Printable A",
  "sources": [
    {
      "type": "attachment",
      "content": {
        "serde": "A"
      },
      "sources": [
        {
          "type": "context",
          "content": "Context C",
          "sources": [
            {
              "type": "backtrace",
              "frames": [],
              "sources": [
                {
                  "type": "spantrace",
                  "frames": [],
                  "sources": [
                    {
                      "type": "context",
                      "content": "Context F",
                      "sources": []
                    }
                  ]
                }
              ]
            },
            {
              "type": "context",
              "content": "Context F",
              "sources": []
            }
          ]
        }
      ]
    }
  ]
}

@TimDiekmann b)
dubbed: flat

[
  {
    "type": "message",
    "content": "Printable A",
    "parent": null,
    "index": 0
  },
  {
    "type": "attachment",
    "content": {
      "serde": "A"
    },
    "parent": 0,
    "index": 1
  }
]

(frames have been omitted)

@indietyp a)
dubbed: hybrid

{
  "messages": [
    "Printable A",
    "Context C"
  ],
  "attachments": [
    {
      "serde": "A"
    }
  ],
  "backtrace": [],
  "spantrace": [],
  "sources": [
    {
      "messages": [
        "Context F"
      ],
      "attachments": [],
      "backtrace": [
        {
          "example": "frame"
        }
      ],
      "spantrace": [
        {
          "example": "frame"
        }
      ]
    },
    {
      "messages": [
        "Context F"
      ],
      "attachments": [],
      "backtrace": [],
      "spantrace": []
    }
  ]
}

[indietyp]

just here so that we have an example for all proposals and versions and all are on the same page.

[indietyp]

simple is definitely the easiest to implement and tries to preserve most of the structure, with flat doing the same without a tree.

I still think that hybrid is the best for most use-cases, but I don't know if it tries... to accomplish too much or is too opinionated. (and if I am too biased)

[TimDiekmann]

I think I misinterpreted your approach. You are collecting a Context and all following Attachment up to the next Context?

Maybe something like this? (replaced the messages with the letters for simplicity:

{
  "context": "C",
  "attachments": [
    "B",
    "A"
  ],
  "sources": [
    {
      "context": "H",
      "attachments": [
        "G",
        "E"
      ],
      "sources": []
    },
    {
      "context": "F",
      "attachments": [],
      "sources": []
    }
  ]
}

NB: Please do not confuse attach_printable with message vs. attach. Without specialization, it's not possible to detect if a parameter is either Display or not, so attach_printable should be the same as attach but also print the content. When specialization hits stable we plan to remove attach_printable. This means, that your approach wouldn't have messages and attachments but only attachments.

[TimDiekmann]

Another hybrid approach which may makes sense: List all frames in a list until we encounter a split:

{
  "frames": [
    {
      "type": "attachment",
      "letter": "A"
    },
    {
      "type": "attachment",
      "letter": "B"
    },
    {
      "type": "context",
      "letter": "C"
    }
  ],
  "sources": [
    {
      "frames": [
        {
          "type": "attachment",
          "letter": "E"
        },
        {
          "type": "attachment",
          "letter": "G"
        },
        {
          "type": "context",
          "letter": "H"
        }
      ],
      "sources": []
    },
    {
      "frames": [
        {
          "type": "context",
          "letter": "F"
        }
      ],
      "sources": []
    }
  ],
}

[indietyp]

I think that's the one I'll go with (if everyone is ok w/ it)

[TimDiekmann]

I wonder if we want to special case span_trace and backtrace (it always annoys me, that "span trace" are two words and "backtrace" is one 🤦🏻‍♂️), As you said it's not really possible currently to get the backtrace frames, but this will probably change in the future. Even though, this will most likely be platform dependant. For span traces, I think there are nice ways of formatting them (if tracing doesn't already have an implementation).

In any case, do we want a special case? Or do we want to provide a hook for serializing a frame as well (similar to the debug/display hook)?

I guess, for now we should not special case anything. Let's implement the basic functionality, so it's actually usable. Later, maybe it's possible to add typed hooks, which request the frames by type (e.g. provide a hook for Diagnostics will output nice diagnostic serde data and when a Frame provides diagnostics, this will be printed). Just some random ideas 🙃

[TimDiekmann]

Maybe this approach could be reused for the Debug implementation?

[indietyp]

(Sorry for the delay, I was on a short holiday^^)

(finally, I am not the only one that is annoyed by the difference between "span trace" and "backtrace")

I'd like to go with the Diagnostics approach. That seems to be the best way forward - w/o any special handling for SpanTrace and Backtrace, but that would only work nightly via demand, so I think my way forward for both would be: to use Provider API nightly and on stable create a special case for Backtrace and SpanTrace.

[TimDiekmann]

No worries, hopefully, you enjoyed it!

Wouldn't it still be possible to get the Backtrace/SpanTrace in a hook by using downcast_ref? I don't know if typed hooks are possible/a good idea, as static generics are not possible in Rust, but this is a topic that needs to be discovered I guess. If the hook function would have a signature like Fn(&Frame, &Serializer) this should work.

[indietyp]

I played around for a bit, and I think I found something. This is using more dyn than I'd like it to do, but considering this is prob. not performance-critical code, it might be fine. This compiles and runs.

https://gist.github.com/indietyp/f092c56b267648b13cd95cab8835e8c3

[indietyp]

This could be made with less dyn, if we allow Hooks to have a generic and then a builder pattern to attach all hooks. Don't know about DX tho. We might also be able to replace erased_serde, but I am not yet sure about that one.