Avoiding issue when unable to add domain

[NickyM]

We were experiencing HTTP 500 when trying to add the following domains:

• solemaids.nl
• solemaids.com.au

The getDomainExpirationDate function returned NULL. The mySQL column isnt nullable. Returning mySQL min date instead.

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[johnnyq]

Hey @NickyM Thank you for the pull request however this should remain NULL as this will throw off our expire checks that we have throughout the code and will produce some unintended results. Basically the expire date should be NULL if no date is given.

[NickyM]

@johnnyq - if it's NULL the server throws a HTTP 500 because the mySQL table isn't nullable.

[wrongecho]

Hey @NickyM!

Thanks for the PR :)

Some background: The format for domain whois data varies on each TLD. We use http://lookup.itflow.org:8080/ for domain expiration date parsing. This hosts a Python service, but doesn't seem to cover all TLDs currently.

If we add these to https://github.com/itflow-org/itflow/issues/869 we can look into fixing the upstream library at some point. I raised a PR recently as part of this post.

--

It looks like we need some better error handling to be honest. It shouldn't give you an error 500, it should just not parse the expiration date and fail gracefully. I'm not sure when this was changed to be "NULL" but given that it's a SQL DATE field that change probably wasn't the best either.

Personally, I'm not opposed to allowing NULL values for this field and properly storing the NULL as you've proposed. It's better than refusing to allow the domain altogether. Yes, it might break expiration alerts but that's easy enough to fix as well.

[wrongecho]

Looking into this further, just running a command-line whois against .be, .nl and .com.au domains, I can't even see a expiry date in the whois data that is returned? Doesn't seem these not publicly shown for these TLDs, so will not look into any further at this time.

--

I've raised a separate PR to properly account for null values. Very similar to your PR @NickyM, but has some improved logic and should cover the cron bits off too.

If you get a moment, please play around on the PR Review page and see if you can break it?

[wrongecho]

PR #888 has now fixed this. Big thanks to @NickyM for raising this and collaborating towards a fix :)