Add safelog annotations to SerializableError and RemoteException

[ash211]

Before this PR

Fields on SerializableError and RemoteException don't have @Safe and @Unsafe logging annotations.

After this PR

==COMMIT_MSG==
Add safelog annotations to SerializableError and RemoteException
==COMMIT_MSG==

Possible downsides?

When adding log safety annotations to widely-used classes, it can require changes in downstream projects. This is good (flushes out log safety issues) but also bad (blocks excavators, requires dev time to fix). Adding safety annotations is overall a net-good though.

[changelog-app]

Generate changelog in changelog/@unreleased

What do the change types mean?

• feature: A new feature of the service.
• improvement: An incremental improvement in the functionality or operation of the service.
• fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
• break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
  and external consumers of the service's API (e.g. customer-written software or integrations).
• deprecation: Advertises the intention to remove service functionality without any change to the
  operation of the service itself.
• manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
  performing database surgery, ...) at the time of upgrade for it to succeed.
• migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

• ❗The break and manual task changelog types will result in a major release!
• 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
• ✨ All others will result in a minor version release.

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

Add safelog annotations to SerializableError and RemoteException

Check the box to generate changelog(s)

• Generate changelog entry

[ash211]

This is problematic. The vast majority of the time, an ErrorName is known at compile time and is safe to log. Example value: Product:SomethingBroke.

However, there is a legacy use of the SerializableError that explains why it has deprecated methods getMessage() and getExceptionClass(). At some point in the past, this class must have corresponded more closely with a Java Exception. But now it is broader, and those fields are deprecated in the builder.

Besides marking these methods as @Deprecated, they are also configured to be skipped at read time: access = JsonProperty.Access.WRITE_ONLY, meaning that services receiving JSON objects with message or exceptionClass fields ignore them at read time. The new test testSerDeRoundTripDropsMessage() shows this.

The message is plumbed through as a default to errorName though. See the default implementation of SerializableError.errorName(). This means if errorName isn't set on an object, then the message is used instead. The new test testLegacyMessageUsedAsErrorNameWhenNoErrorNameIsSet() shows this.

Because messages are unsafe, this means the errorName can be unsafe as well. In practice this only happens for SerializableError objects, created using the legacy builder methods that have been marked deprecated, and not yet serialized to a JSON object (still in memory).

---

I don't think we want to change this line of code to have error name be an Unsafe arg though. I'd rather get it to be Safe 100% of the time. This needs to be done before merging this PR as-is (unless we suppress..)

To prevent message strings from reaching the error name field and causing them to become unsafe, I propose that we keep the Immutables method for setting the message on the ImmutableSerializableError.Builder, but ignore it and replace with a fixed error name, like Default:EmptyErrorNameWithLegacyMessageUsage.

Callers can continue calling .message() on the builder, but it doesn't show anywhere.

[ash211]

My proposed fix for this is at #1054

[pkoenig10]

I think we can just ignore this case. No one should be using servers that emit the legacy error format. It's reasonable to assume that error names are safe.

[pkoenig10]

We probably should annotation parameters with @Unsafe.

Although I suspect this will be highly disruptive. There are many places where clients plumb these parameters into their own, safe, error args.