Skip to content

Gracefully handle unknown fields#316

Merged
Rafal Niski (rafalniski) merged 4 commits intocontentful:masterfrom
zeatful:gracefully-handle-unknown-fields
Jun 20, 2025
Merged

Gracefully handle unknown fields#316
Rafal Niski (rafalniski) merged 4 commits intocontentful:masterfrom
zeatful:gracefully-handle-unknown-fields

Conversation

@zeatful
Copy link
Copy Markdown
Contributor

@zeatful zeatful (zeatful) commented Mar 28, 2025

We have had the following issues arise a few times now:

  • a User updates a Content Model to add a new field
  • a User updates a piece of Content and adds data for that new field
  • Our application has not been updated to know about that new field and the SDK then errors:
processing failed: java.lang.IllegalArgumentException: Can not set com.cms.model.content.components.layout.ComponentConfiguration field com.cms.model.content.components.layout.PageLayoutComponent.componentConfiguration to com.google.gson.internal.LinkedTreeMap] with root cause",
  "logger_name": "org.apache.catalina.core.ContainerBase.[Tomcat].[localhost].[/].[dispatcherServlet]",
  "thread_name": "http-nio-8080-exec-9",
  "level": "ERROR",
  "level_value": 40000,
  "stack_trace": "
java.lang.IllegalArgumentException: Can not set com.cms.model.content.components.layout.ComponentConfiguration
field com.cms.model.content.components.layout.PageLayoutComponent.componentConfiguration to 
com.google.gson.internal.LinkedTreeMap
at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
at java.base/jdk.internal.reflect.UnsafeObjectFieldAccessorImpl.set(UnsafeObjectFieldAccessorImpl.java:81)
at java.base/java.lang.reflect.Field.set(Field.java:799)
at com.contentful.java.cda.TransformQuery.transformFieldAnnotation(TransformQuery.java:482)
at com.contentful.java.cda.TransformQuery.transform(TransformQuery.java:412)
at com.contentful.java.cda.TransformQuery.transformFieldAnnotation(TransformQuery.java:471)
at com.contentful.java.cda.TransformQuery.transform(TransformQuery.java:393)
at com.contentful.java.cda.TransformQuery.transformFieldAnnotation(TransformQuery.java:459)
at com.contentful.java.cda.TransformQuery.transform(TransformQuery.java:393)
at com.contentful.java.cda.TransformQuery.access$000(TransformQuery.java:28)
at com.contentful.java.cda.TransformQuery$4.apply(TransformQuery.java:321)
at com.contentful.java.cda.TransformQuery$4.apply(TransformQuery.java:312)
at io.reactivex.rxjava3.internal.operators.flowable.FlowableMap$MapSubscriber.onNext(FlowableMap.java:64)

As the code appears to be to parse the CDAEntry and assume that a corresponding field exists on the Result (DTO representation in our code) using reflection to directly call the setter, it opens up the opportunity for this to potentially happen for content fields that are not defined in the DTO model representation.

This suggestion is to ignore those exceptions and continue on with what we do know how to handle. I don't believe we should have to denote EVERY single field explicitly on our side to have the CDAClient transform correctly.

If this is not desired, perhaps this can be gated behind a property on the CDAClient to let users decide? I'm also open to proper logging behavior for this as I do not see much logging within this client for situations like this.

@zeatful zeatful (zeatful) requested a review from a team as a code owner March 28, 2025 21:26
@ce-angi
Copy link
Copy Markdown

ce-angi commented Apr 1, 2025

Just noting, thanks for working on this! Will definitely help reduce errors in our application.

@EliasSadat
Copy link
Copy Markdown

EliasSadat commented Jun 10, 2025

This would be an impactful improvement if it could get reviewed and merged.

@zeatful zeatful (zeatful) requested a review from a team as a code owner June 10, 2025 20:46
@zachariahc
Copy link
Copy Markdown

With this update our organization would experience a reduction in errors. Hopefully this gets merged in.

@rafalniski Rafal Niski (rafalniski) merged commit 7070b5e into contentful:master Jun 20, 2025
4 checks passed
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

Successfully merging this pull request may close these issues.

7 participants