[v9]: Not possible to remove information in beforeSend (make SentryEvent and related classes mutable)

[ColinSchmale]

Platform

Flutter Web

Obfuscation

Enabled

Debug Info

Enabled

Doctor

Version

8.12.0

Steps to Reproduce

Use options.beforeSend to override a value like this:

await Sentry.init((options) {
  options.beforeSend = (event, hint) {
    return event.copyWith(serverName: null); // Don't send server names.
  };
});

Expected Result

The value is not sent (or null).

Actual Result

The value is sent and not null.

This is what the documentation says about how to remove information:

await Sentry.init((options) {
  options.beforeSend = (event, hint) {
    return event.copyWith(serverName: null); // Don't send server names.
  };
});

but when you take a look at the code, this cannot work:

  final String? serverName;

  @override
  SentryEvent copyWith({
    String? serverName,
  }) =>
      SentryEvent(
        serverName: serverName ?? this.serverName,

Replacing the geo object with an empty geo object does also not work. It seems that the data is overwritten afterwards.

await Sentry.init((options) {
  options.beforeSend = (event, hint) {
    return event.copyWith(
        user: event.user?.copyWith(
        geo: SentryGeo(),
      ),
    );
  };
});

What I ended up doing was masking the city value like this:

await Sentry.init((options) {
  options.beforeSend = (event, hint) {
    return event.copyWith(
        user: event.user?.copyWith(
        geo: SentryGeo(city: '****'),
      ),
    );
  };
});

Related to #980

Are you willing to submit a PR?

None

[buenaflor]

hi, thanks for raising this. we'll look into this and see how we can provide an option to make this possible

[buenaflor]

@ColinSchmale I believe setting it to an empty string "" also does the trick

[ColinSchmale]

**** is also fine for me.

I mostly created this issue because the code in the documentation is wrong and maybe to also get some information why this happens and if my solution is okay.

[buenaflor]

yeah we gotta update the docs definitely

it's been like this way before my time but afaict it is intended that way. so your method is right, setting null won't do anything you have to set it to some value e.g empty string or how you did it

I don't think it's ideal from a dx perspective but let's see what we can do there

[buenaflor]

Since there is no practical use for having these immutable we've decided to refactor these into mutable data classes for the next major v9

[denrase]

Gave this some thought and I think there might also be some risk in changing our immutable data objects. Right now they are thread safe and also hashable, so this would be large a behavioural change. Also having them final is the recommended way: accoridng to Effective Dart Docs.

There would be another way, ut this would change the way we use copyWith.

// Generic wrapper to preserve type information
class Value<T> {
  final T? value;
  const Value(this.value);
  const Value.absent() : value = null;
}

class Person {
  final String? name;
  final int? age;

  const Person({this.name, this.age});

  Person copyWith({
    Value<String>? name,
    Value<int>? age,
  }) {
    return Person(
      name: name != null ? name.value : this.name,
      age: age != null ? age.value : this.age,
    );
  }
}

// Usage
final person = Person(name: "John", age: 30);
final updatedPerson = person.copyWith(
  name: Value(null),  // Sets name to null
  // age remains unchanged
);

[denrase]

Or we could use nullable functions as parameters, either replacing copyWith or adding an additional method, optionally deprecating copyWith.

class Person {
  final String? name;
  final int? age;

  const Person({this.name, this.age});

  // Original copyWith remains unchanged
  Person copyWith({
    String? name,
    int? age,
  }) {
    return Person(
      name: name ?? this.name,
      age: age ?? this.age,
    );
  }

  // Renamed method that supports setting nulls
  Person nullableCopyWith({
    String? Function()? name,
    int? Function()? age,
  }) => copyWith(
    name: name?.call(),
    age: age?.call(),
  );
}

// Usage
final person = Person(name: "John", age: 30);
final updated1 = person.copyWith(name: "Jane");     // Original usage
final updated2 = person.nullableCopyWith(           // New usage for null setting
  name: () => null,
  age: () => 31,
);

[denrase]

Updated docs: getsentry/sentry-docs#12832

[ColinSchmale]

I agree that immutable data objects are better. The solution with the nullable functions as parameters is pretty nice but wouldn't be necessary if this worked:

await Sentry.init((options) {
  options.beforeSend = (event, hint) {
    return event.copyWith(
        user: event.user?.copyWith(
        geo: SentryGeo(),
      ),
    );
  };
});

Do you know why I wasn't able to remove the geo data by overriding geo with a new object? All values of the new SentryGeo() object should be null, right?

[stefanosiano]

hi @ColinSchmale
The discussion above is for a more general approach into how we can provide a way to modify any value.
However, at a glance, your workaround should work fine.
We'll definitely take a look into why it's not working

[denrase]

@ColinSchmale Thx for the update! Setting a new geo object should work, i'll investigate this.

[denrase]

@ColinSchmale So just from the clients perspective, i can set an empty geo object to the user. The geo is empty, so no payload is sent. So i'd say this works as expected from the clients perspective. Could you give us more context on how to reproduce this?

[denrase]

@ColinSchmale @stefanosiano Only thing i can imagine is, since we do not send empty geo json, the backend is also "ignoring" it. But I'm missing context here about what is supposed to be the outcome.

[buenaflor]

Gave this some thought and I think there might also be some risk in changing our immutable data objects. Right now they are thread safe and also hashable, so this would be large a behavioural change. Also having them final is the recommended way: accoridng to Effective Dart Docs.

• thread safe -> correct me if I'm wrong but I don't think this really is an issue in dart since dart is singled threaded and isolates dont share memory with one another
• hashable -> what functionality in the SDK could break here?
• having them final is the recommended way -> imo this is not a Dart specific thing, you could argue all our other SDKs should do this as well but we are as far as I can tell the only SDK doing it this way.

imo we have a good use case here since we do have to mutate the event frequently

I feel like adding more and more stuff on top to mak setting fields nullabe work seems to me is just adding more complexity compared to having it mutable. of course changing the implementation to mutable will also entail its own workload but imo worth it especially now with an upcoming major it's a good opportunity to improve this