Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserialization error with generically-typed events #1069

Closed
eouw0o83hf opened this issue Aug 13, 2018 · 3 comments
Closed

Deserialization error with generically-typed events #1069

eouw0o83hf opened this issue Aug 13, 2018 · 3 comments
Milestone

Comments

@eouw0o83hf
Copy link
Contributor

eouw0o83hf commented Aug 13, 2018

I'm trying to add metadata to events (see #780, IMO this should be supported by the framework at a basic level), and went the route of adding a wrapper around each event, as such:

public class Envelope<T>
{
    public T Value { get; set; }
    public Guid ExecutingUserId { get; set; }
}

public class Created
{
    public Guid Id { get; set; }
}

public class Updated
{
    public String UpdateValue { get; set; }
}

Code to manage event streams:

public async Task StartStream<TEvent>(Guid streamId, TEvent _event)
{
    using(var session = _store.OpenSession())
    {
        session.Events.StartStream(streamId, (object)new Envelope<TEvent>
        {
            Value = _event,
            ExecutingUserId = GetUserIdFromSomewhere()
        });
        await session.SaveChangesAsync();
    }
}

public async Task Commit<TEvent>(Guid streamId, TEvent _event)
{
    using(var session = _store.OpenSession())
    {
        session.Events.Append(streamId, (object)new Envelope<TEvent>
        {
            Value = _event,
            ExecutingUserId = GetUserIdFromSomewhere()
        });
        await session.SaveChangesAsync();
    }
}

Events commit happily, updated projections correctly, and even persist the correct event type to the database.

Querying the event stream throws an exception though. Stream loading code:

public async Task<IReadOnlyList<IEvent>> LoadStream(Guid id)
{
    using(var session = _store.OpenSession())
    {
        return await session.Events.FetchStreamAsync(id);
    }
}

Attempting to load a stream with two different events (Envelope<Created> and Envelope<Updated>) results in all events attempting to be deserialized using the type of the first event (Envelope<Created>).

The error that surfaces is a JSON parsing issue (JsonReaderException: Unexpected character encountered while parsing value), but I ran intermediate code under the JSON parser and discovered that the second event is attempting to be deserialized using the Type of the first event.

Troubleshooting I've done:

  • Fetch just the second event (Envelope<Updated>) to verify that there's no inherent problem with that record. Using QueryAllRawEvents() and skipping the first record, it deserializes correctly and I get a nice Envelope<Updated>
  • Remove my Envelope<> wrapper and just persist the events (maybe my code is the problem). This works, which means that the problem is probably down in the Marten framework itself.

Hopefully that's useful debugging information - if you can point me in the right direction I may have a go at fixing it myself.

@jeremydmiller
Copy link
Member

@eouw0o83hf It's not a serialization error per se, it's just that Marten is probably being too naive about how it stores the event type name. Closed generic event types really wasn't something we anticipated here. That's really not a case I'm excited about supporting because the reflection wok around that is always a huge mess.

It'd be easier to deal with this with a simple layer supertype for your event classes, even without waiting for us to add the event metadata

@jeremydmiller jeremydmiller added this to the 2.10.0 milestone Aug 17, 2018
@eouw0o83hf
Copy link
Contributor Author

@jeremydmiller I'll do some more research, but I seem to remember getting correct types out if I queried the data in different ways, which led me to believe that Types are being persisted correctly but maybe reinstantiated naively.

I wound up going the supertype route, it's probably an overall less-complex way to deal with it.

I have some time coming up to get the Marten solution setup locally, I'll see if I can knock this out.

@jeremydmiller
Copy link
Member

Fixed in a local branch. This will be in Marten 2.10.0 and 3-alpha-2 tomorrow (8/22)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants