italia / spid-sp-test

SAML2 SPID/CIE Service Provider validation tool
European Union Public License 1.2
38 stars 17 forks source link

Unhandled exceptions #165

Closed damikael closed 11 months ago

damikael commented 11 months ago

When executed in a batch loop, sometimes, spid-sp-test crashes with the following output/error:

env IDP_ENTITYID="https://validator-test.spid.gov.it"  spid_sp_test  --metadata-url file://metadata.xml  --profile spid-sp-ag-public-full --debug ERROR --production -rf json -o metadata-extra.json --extra

[ERR] null
[STDOUT] Spid QA: executed 323 tests, 0 failed. 0 warnings.
[STDERR] CRITICAL:__main__:Errors occourred during Check, <class 'spid_sp_test.metadata_extra.SpidSpMetadataCheckExtra'>: time data '' does not match format '%b %d %H:%M:%S %Y' 

Instead the output when the command is executed out of the loop is:

env IDP_ENTITYID="https://validator-test.spid.gov.it"  spid_sp_test  --metadata-url file://metadata.xml  --profile spid-sp-ag-public-full --debug ERROR --production -rf json -o metadata-extra.json --extra

[ERR] ERROR:spid_sp_test.metadata:If the Company ->COMPANY<- element is present it MUST be equal to OrganizationName ->Company<-
ERROR:spid_sp_test.metadata:SPID Compliant Certificates Checking basicConstraints x509 extension critical error
[STDOUT] Spid QA: executed 404 tests, 2 failed. 0 warnings.
[STDERR] 
peppelinux commented 11 months ago

the first exception is excplicit related to time data '' does not match format '%b %d %H:%M:%S %Y' and should be handled in the SpidSpMetadataCheckExtra, exactly here:

https://github.com/italia/spid-sp-test/blob/main/src/spid_sp_test/metadata_extra.py#L106C34-L106C34

I can review and approve a PR if possibile

the other exception is related to spid-compliant-certificates-python and it seems not related to me to the first issue, anyway we should patch the first one, doing tests and then move to spid-compliant-certificates-python if you agree

damikael commented 11 months ago

The second output is the expected. The problem is that when the command crashes due to unhandled exceptions the returned err code is null, as for success execution, so it cannot let to know if the command worked or failed.

peppelinux commented 11 months ago

PR merged @damikael

can you provide a check for absent datetime here? https://github.com/italia/spid-sp-test/issues/165#issuecomment-1780878658

we may release a new version with this I hope

damikael commented 11 months ago

https://github.com/italia/spid-sp-test/pull/167

peppelinux commented 11 months ago

Resolved by https://github.com/italia/spid-sp-test/releases/tag/v1.2.14