search

fkie-cad / fact_extractor

Standalone Utility for FACT-like extraction
GNU General Public License v3.0
80 stars 31 forks source link

Add exceptions to plugins; fix unit tests where necessary #13

Closed svidovich closed 4 years ago

svidovich commented 5 years ago

The exceptions will be handled by the unpack logic, and so this should work just the same as before with some better logging. In addition, return code metadata will be made available. I also put some effort into 'standardizing' the plugins -- the ones changed now output logging and have a more unified format.

GenericFS and Sevenz seem to have more moving parts than the others, so I avoided the conversion for those two for now. I tried to prep GenericFS for the future change a little bit.

dorpvom commented 5 years ago

I'd prefer a more specific exception here. Either a custom such as UnpackError or a more specific builtin though I'm not sure what would fit here since there is no such thing as a NonZeroReturnError or similar.

Right now this drops to

except Exception as e:
    logging.debug('Unpacking of {} failed: {}: {}'.format(file_path, type(e), str(e)))
    additional_meta = {'error': '{}: {}'.format(type(e), str(e))}

where a debug is printed - duplicating data from the debugs introduced here - and the putting a generic error report around the given exception. Specifically, the plugin meta data is not processed.

When building a custom exception, you can pass the already generated meta data as parameter and then catch it here to forward the values. In this case, you do not loose semantic while introducing the new one.

Something like:

except CustomNonZeroError as error:
    additional_meta = error.unpacker_meta if error.unpacker_meta else {}
    additional_meta['error'] = str(error)
except Exception as e:
    ...
svidovich commented 5 years ago

I haven't forgotten! I'll be working on this soon.

dorpvom commented 4 years ago

I haven't forgotten! I'll be working on this soon.

You still planning on doing this? Otherwise I'll close this PR and table the feature.

svidovich commented 4 years ago

No, unfortunately not. Sorry about that