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

Improve reload diagnosics when a character set plugin is missing #1282

Merged

Conversation

stevedlawrence
Copy link
Member

Currently, reloading a saved parser when a neccessary character set plugin is not on the classpath results in an exception with a very helpful message that has nothing to do with character sets or plugins, for example:

java.lang.ClassCastException: cannot assign instance of
scala.collection.immutable.List$SerializationProxy to field
org.apache.daffodil.runtime1.processors.ModelGroupRuntimeData.groupMembers
of type scala.collection.Seq in instance of
org.apache.daffodil.runtime1.processors.SequenceRuntimeData

This is a known issue with scala and its use of proxy when serializing data structures like Lists and Maps.

To fix the exception and improve diagnostics, this uses writeReplace to serialize a BitsCharset as a BitsCharsetSerializationProxy with information about the needed BitsCharset. This proxy implements readResolve to look up the BitsCharset from the bits charset registry and restore the original BitsChraset. If not found in the registry, then we throw an helpful exception that bubbles up to the reload function and we can output a helpful diagnostic. Now we get something like:

[error] The saved parser was created with a different set of
dependencies containing a class no longer on the classpath: Charset
plugin com.example.MyCustomCharset for X-DFDL-MY-CUSTOM-CHARSET

Add an integraion test to make sure this errors with a reasonable diagnostic, which require splitting a schema file into valid and invalid parts so we can reuse it with the CLI.

DAFFODIL-2915

@stevedlawrence stevedlawrence force-pushed the daffodil-2915-reload-missing-plugin branch from 8ca4f65 to 8890738 Compare August 22, 2024 20:05
@stevedlawrence
Copy link
Member Author

I thought it would probably be wise to add tests that reload a saved parser with missing layers and UDFs on the classpath, in addition to charsets. UDF's work as is and give a good diagnostic. Layers also work, but create an SDE that must be caught when reloading or else the CLI thinks it's a bug. PR is upated.

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor

@jadams-tresys jadams-tresys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

This definitely would have saved me some headaches when trying to figure out why a schema stopped working all of a sudden when I didn't realize the charset was no longer in the JAVA/DAFFODIL path.

Currently, reloading a saved parser when a neccessary character set
plugin is not on the classpath results in an exception with a very
helpful message that has nothing to do with character sets or plugins,
for example:

    java.lang.ClassCastException: cannot assign instance of
    scala.collection.immutable.List$SerializationProxy to field
    org.apache.daffodil.runtime1.processors.ModelGroupRuntimeData.groupMembers
    of type scala.collection.Seq in instance of
    org.apache.daffodil.runtime1.processors.SequenceRuntimeData

This is a known issue with scala and its use of proxy when serializing
data structures like Lists and Maps.

To fix the exception and improve diagnostics, this uses writeReplace to
serialize a BitsCharset as a BitsCharsetSerializationProxy with
information about the needed BitsCharset. This proxy implements
readResolve to look up the BitsCharset from the bits charset registry
and restore the original BitsChraset. If not found in the registry, then
we throw an helpful exception that bubbles up to the reload function and
we can output a helpful diagnostic. Now we get something like:

    [error] The saved parser was created with a different set of
    dependencies containing a class no longer on the classpath: Charset
    plugin com.example.MyCustomCharset for X-DFDL-MY-CUSTOM-CHARSET

Add an integraion test to make sure this errors with a reasonable
diagnostic, which require splitting a schema file into valid and invalid
parts so we can reuse it with the CLI.

Add tests for reloading UDFs and layers without a correct classpath.
UDF's work as is. For layers we just need to catch SDE exception when
reloading.

DAFFODIL-2915
@stevedlawrence stevedlawrence force-pushed the daffodil-2915-reload-missing-plugin branch from 51af1d0 to eeea03b Compare August 27, 2024 19:30
@stevedlawrence stevedlawrence merged commit be3fee7 into apache:main Aug 27, 2024
11 checks passed
@stevedlawrence stevedlawrence deleted the daffodil-2915-reload-missing-plugin branch August 27, 2024 21:32
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.

3 participants