AshPaperTrail Crash: ForbiddenField & Full Diff Bug

by Admin 52 views
AshPaperTrail Crash: ForbiddenField & Full Diff Bug

The full_diff Crash: A Deep Dive into AshPaperTrail and Forbidden Fields

Hey guys! 👋 I've got a tricky situation here. I recently bumped into a nasty crash while working with AshPaperTrail in my Elixir/Ash project, and I wanted to share my findings and hopefully get some help. The issue revolves around how AshPaperTrail handles updates when field_policies are in play, specifically when a %Ash.ForbiddenField{} sneaks into the changeset during an update mutation using AshGraphql. Let's dive in! This situation involves several key components, including Elixir, Erlang, Ash, AshPaperTrail, AshGraphql, and of course, the ever-present full_diff change builder.

First, let's set the stage. I'm using Elixir 1.19.1, Erlang 28.1, Ash 3.5.43, and ash_paper_trail main. I'm developing on MacOS. The core of the problem lies within an update mutation on my GF.Events.Participation2 resource. I added field_policies to this resource, which, as you may know, restricts what data can be updated under certain conditions. The problem arises when an update mutation attempts to modify the Participation2 record. The mutation delegates to a complex change module. A new Participation2 record is generated as part of a feature that allows transferring event tickets. When this update is triggered, it throws an Ash.Error.Unknown error. This happens when passing a changeset to AshPaperTrail.Resource.Changes.CreateNewVersion.change/3 that includes a %Ash.ForbiddenField{}. This is where the full_diff change builder comes into play.

Now, let's talk about the error log. The error log is pretty verbose, but the relevant bits are in the stack trace. The crucial line to note in the error log is this: ** (MatchError) no match of right hand side value: :error. This MatchError is triggered within the AshPaperTrail.ChangeBuilders.FullDiff.Helpers.dump_value/2 function. It seems the full_diff change builder, which is responsible for creating a detailed diff of the changes, is failing to handle the presence of a %Ash.ForbiddenField{} in the changeset's data. I believe the issue arises because the dump_value function isn't prepared to gracefully handle a %Ash.ForbiddenField{} structure, leading to the MatchError. This function is part of the machinery that AshPaperTrail uses to build the change history. The full_diff builder tries to create a complete picture of all the changes that have happened. It takes the old and new values and compares them to figure out exactly what was updated.

Debugging the changeset Value

Alright, let's get into the nitty-gritty of the changeset that's causing all the fuss. I've included a snippet from the error log showing the value of the changeset passed to the CreateNewVersion action. This is a critical piece of the puzzle. The changeset is essentially a data structure that holds all the information about the changes being made. The error arises during an update operation, where a record is modified. The debug output of the changeset shows a lot of detail, but the key part for us is the :data section. This is where the actual data of the record resides. Within the :data struct, you'll see a field called :note, and here's the kicker: it contains an #Ash.ForbiddenField{}. This is the smoking gun! The presence of a ForbiddenField indicates a field that, due to field policies, cannot be directly modified through the current update operation. The issue is that the full_diff change builder doesn't know how to handle these ForbiddenField values correctly. When it tries to dump_value of the :note field, it crashes because it's not expecting a ForbiddenField. This is why the MatchError is triggered. The unexpected presence of the ForbiddenField is what causes the crash. Understanding this changeset and its contents is key to understanding the error.

The Context and Reproduction of the Issue

Okay, let's talk context, and how to make this happen. The context here is an update mutation within a GraphQL operation using AshGraphql. This mutation delegates to a complex change module. My mutation handles the transfer of an event ticket to another person. It fetches the existing Participation2 record, generates a new one, and then updates the existing one. The core of this issue comes from how the full_diff change builder in AshPaperTrail handles changesets that contain ForbiddenField values. Specifically, it seems the dump_value/2 function is not prepared to process such values, resulting in a MatchError when it encounters them during the diff creation process. The problem can be reproduced by passing a changeset to AshPaperTrail.Resource.Changes.CreateNewVersion.change/3 that includes a %Ash.ForbiddenField{} value for one of the attributes in the :data struct. The ticket transfer feature is a good example of how this can occur. Here’s a simplified breakdown:

  1. A user initiates a ticket transfer. The ticket transfer logic begins.
  2. The existing Participation2 record is fetched.
  3. A new Participation2 record is created (e.g., with the new owner's info).
  4. The old record is updated with the new Participation2 data and additional details, resulting in a changeset that may contain forbidden fields (like :note).
  5. When AshPaperTrail tries to create the version diff using full_diff, it encounters the ForbiddenField in the changeset and crashes.

In essence, the full_diff change builder is not prepared to handle forbidden fields. The root cause is the dump_value function not knowing how to manage the ForbiddenField structure in the changeset. To reproduce this issue, you need a scenario where a field is forbidden from being directly updated but is still present in the changeset.

Expected Behavior and Possible Solutions

So, what's the expected behavior, and how can we get this fixed? Ideally, AshPaperTrail should gracefully handle the presence of a %Ash.ForbiddenField{} in a changeset. Regardless of field policies, the diff should process without crashing. Now, I'm not entirely sure whether a ForbiddenField value in the changeset data should be considered valid or not. But at the very least, it shouldn't crash the application. Here are a couple of potential solutions:

  1. Modify dump_value/2: The dump_value/2 function in AshPaperTrail.ChangeBuilders.FullDiff.Helpers needs to be updated to handle the %Ash.ForbiddenField{} case. This might involve adding a clause to explicitly check for Ash.ForbiddenField and either skip it, log a warning, or represent it in a way that doesn't cause a MatchError. This is probably the most direct solution.

  2. Filter ForbiddenField from Changesets: Before passing the changeset to AshPaperTrail, the code could filter out or transform any ForbiddenField values. This would ensure that the full_diff builder only deals with valid data. This approach is less elegant, but it is a potential workaround.

  3. Improve Error Handling: Even if the underlying issue is not immediately addressed, improve the error handling in AshPaperTrail. Instead of crashing with a MatchError, more graceful behavior, like logging a more informative error message, could be implemented.

From my point of view, the best approach would be to modify the dump_value/2 function to handle the %Ash.ForbiddenField{} struct correctly. This way, the diff creation process can continue without crashing, and the change history will accurately reflect what changed, even with field policies in place. I'm keen to hear your thoughts and any other insights on how to handle this. If you are a library maintainer, please consider this to be a bug report. Thanks, guys!