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

GDS Fails To Launch With Unknown Types In Dictionary #2230

Open
LeStarch opened this issue Aug 23, 2023 · 7 comments
Open

GDS Fails To Launch With Unknown Types In Dictionary #2230

LeStarch opened this issue Aug 23, 2023 · 7 comments
Labels
bug Easy First Issue An issue that should be straight forward to implement, and easily tested via CI. F´ GDS Issues pertaining to the F´ GDS help wanted High Priority High Priority issue that needs to be resolved.

Comments

@LeStarch
Copy link
Collaborator

F´ Version all

Problem Description

The GDS crashes on launch when provided with a dictionary containing references to a handcoded type. See: #1643 (comment)

This may also apply to other complicated types like serlizables.

Expected Behavior

Instead of failing to launch, the GDS should launch but warn that it's unable to deserialize EVRs referencing the handcoded type.

@LeStarch LeStarch added the bug label Aug 23, 2023
@LeStarch
Copy link
Collaborator Author

From: @JackNWhite

I am trying out this modification.

diff --git a/src/fprime_gds/common/loaders/xml_loader.py b/src/fprime_gds/common/loaders/xml_loader.py
index b7e8984..4cc0e0c 100644
--- a/src/fprime_gds/common/loaders/xml_loader.py
+++ b/src/fprime_gds/common/loaders/xml_loader.py
@@ -180,7 +180,13 @@ class XmlLoader(dict_loader.DictLoader):
 
                 arg_name = arg_dict[self.ARG_NAME_TAG]
                 arg_type_name = arg_dict[self.ARG_TYPE_TAG]
-                arg_typ_obj = self.parse_type(arg_type_name, arg, xml_tree, context)
+
+                try:
+                    arg_typ_obj = self.parse_type(arg_type_name, arg, xml_tree, context)
+
+                # Return nothing if the args are at least partly nonsense
+                except exceptions.GseControllerParsingException:
+                    return []
 
                 arg_desc = None
                 if self.ARG_DESC_TAG in arg_dict:

@thomas-bc thomas-bc added the F´ GDS Issues pertaining to the F´ GDS label Aug 24, 2023
@LeStarch LeStarch added High Priority High Priority issue that needs to be resolved. help wanted Easy First Issue An issue that should be straight forward to implement, and easily tested via CI. labels Mar 27, 2024
@AlesKus
Copy link
Contributor

AlesKus commented Dec 23, 2024

Is the issue still valid?
I'm trying to reproduce it. At the moment, I didn't manage to find a clear documentation about defining a handcoded type via xml. At the same time, the fpp does not allow to use "not displayable" types for telemetry and events:
изображение

@matt392code
Copy link
Contributor

This would:

  • Prevent crashes
  • Log warnings about unparseable types
  • Maintain type information for debugging
  • Allow GDS to continue operating with other valid types
try:
    arg_typ_obj = self.parse_type(arg_type_name, arg, xml_tree, context)
except exceptions.GseControllerParsingException as e:
    LOGGER.warning(f"Unable to parse type {arg_type_name}: {str(e)}")
    # Create placeholder type object that indicates unparseable data
    arg_typ_obj = UnparseableType(arg_type_name)

@bocchino
Copy link
Collaborator

bocchino commented Jan 7, 2025

I think we will need something like this. Non-displayable types can appear in data product records, which aren't displayed in the real-time GDS output. @jwest115 discovered that these types are not correctly handled in the JSON dictionary. To fix this problem, we will need to represent non-displayable types in the dictionary, and the GDS will have to ignore them.

@matt392code
Copy link
Contributor

Proposed approach for representing non-displayable types:
non-displayable-types.py.txt
This implementation provides several key features:

  1. A NonDisplayableType class that:

    • Inherits from BaseType to maintain compatibility
    • Stores basic type information (name, size if known)
    • Provides serialization/deserialization capabilities
    • Returns placeholder values when data access is attempted
    • Logs appropriate debug messages
  2. Enhanced XML parsing that:

    • Attempts to extract size and format information when available
    • Falls back gracefully when type information is incomplete
    • Maintains type information in the dictionary
  3. Modified exception handling that:

    • Converts unparseable types to NonDisplayableType instances
    • Preserves type information for debugging
    • Allows GDS to continue operating with other valid types

To use this solution:

  1. Add the NonDisplayableType class to the GDS type system
  2. Modify XmlLoader to implement the enhanced parse_type method
  3. Update dictionary serialization to handle NonDisplayableType instances

This approach would solve the original issue while:

  • Preventing GDS crashes on launch
  • Maintaining type information for debugging
  • Allowing data product records to reference non-displayable types
  • Providing clear logging about unparseable types
  • Keeping the system extensible for future type handling needs

@LeStarch
Copy link
Collaborator Author

@jwest115 are you addressing this as part of your work?

@bocchino
Copy link
Collaborator

bocchino commented Jan 23, 2025

To fix this problem, we will need to represent non-displayable types in the dictionary, and the GDS will have to ignore them.

We decided to exclude non-displayable types from records in FPP, because they have no size information that is discoverable by the analyzer. See nasa/fpp#576. So allowing undefined types in the dictionary is not currently in the scope of planned work. In the current design, if a type appears in the dictionary and it has no definition, then the dictionary is malformed.

Also, I agree that ideally the GDS shouldn't crash if it's given malformed JSON input. However, do we think that an undefined type is the only problem that could cause the GDS to crash? I suspect that hardening the GDS against malformed JSON would require more error checking and testing than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Easy First Issue An issue that should be straight forward to implement, and easily tested via CI. F´ GDS Issues pertaining to the F´ GDS help wanted High Priority High Priority issue that needs to be resolved.
Projects
None yet
Development

No branches or pull requests

5 participants