nasa/opera-sds-pge

[Bug]: PGE/SAS log not committed to disk when uncaught exception is thrown during product metadata collection

Closed this issue · 2 comments

Checked for duplicates

Yes - I've already checked

Describe the bug

While troubleshooting an unrelated error for the DISP-S1 PGE, I noticed that when the PGE raises an exception during product metadata collection (used for ISO XML generation) that said exception goes unhandled. This is undesirable, since it means the combined PGE/SAS log is not written to disk beforehand, resulting in lost info that could be useful when debugging other issues.

What did you expect?

Each of our PGE classes should define a _collect_<pge_name>_product_metadata() method, which is responsible for calling a function from either tiff_utils.py or h5_utils.py that performs the product-specific steps of metadata collection from an output product.

These calls to the product-specific metadata collection functions need to be wrapped in try/except blocks that catch any exception, and then issue a logger.critical call to ensure that we commit the log to disk before allowing the exception to propagate up the call stack.

For example:

def _collect_disp_s1_product_metadata(self, disp_product):
     """
     ...
     """
     try:
         # Extract all metadata assigned by the SAS at product creation time
         output_product_metadata = get_disp_s1_product_metadata(disp_product)
     except Exception as err:
         self.logger.critical( ... )

     ...

This pattern should be applied to all the metadata collection functions within each PGE module.

Reproducible steps

1.
2.
3.
...

Environment

- Version of this software [e.g. vX.Y.Z]
- Operating System: [e.g. MacOSX with Docker Desktop vX.Y]
...

In looking through disp_s1_pge.py for the call to _collect_disp_s1_product_metadata() i see that it is called from _core_filename() and I think we should clarify what is going on in terms of collecting the metadata. I see why it is done in this method since the generation of the data needs the inter_disp_product_filename. However, I think we have to call out that the _core_filename() method is doing more that just returning a filename.

Closed by #505