It should also be noted that Facter 4 ended up keeping the legacy facts. However, we should still update to the modern facts so at some point legacy facts can be turned off.

from the other issue:

Today we still support so called legacy facts in places we parse puppet facts. The drop of the Facter 2 was announced a while ago at https://community.theforeman.org/t/the-road-to-making-puppet-optional/17983 and Foreman 2.6/3.0 would be a good time to clean up this code

While some work has been done, there are still facter 2 facts supported, they should be removed from the codebase.

Guys, are we sure about dropping facter 2.x from the codebase? There was a 2.4 user complaining about 2.x facter broken after upgrade, I think we might still have some users. And this should probably be communicated in advance, we haven't announced or deprecated anything yet IIRC.

https://community.theforeman.org/t/puppet-fact-parser-is-failing-with-string-does-not-have-dig-method/23138

From discovery standpoint we are now good, it took me a while to upgrade to f4 but we are good. But honestly I think we should actually do the very opposite - fix f2 support, then do some announcement and deprecation warnings and schedule this to 3.1 or something like that.

IMHO we should make sure the codebase 100% works when no legacy facts are present. That will be a first step to deprecation.

The particular case mentioned doesn't appear to be related. The structured os fact should be present in 2.4 already.

While I agree, I think we should keep the status-quo for some time before we are ready for the switch. I suggest that we explore what exactly is wrong, since this was broken quite recently (the user reports 2.4) chances are we can fix that easily and buy some more time.

In the linked post I have said that it should work if the user doesn't use stringify_facts = true. In Puppet 4 that setting has been removed and is always false. We have also dropped support for Puppet < 5 so it would be weird to document "this is how you make unsupported software work". Also note that the user reported they upgraded to 1.24 which is also unsupported.

What we can do is more graceful error handling. IMHO this isn't very helpful to a user. I'm thinking about adding abstractions for all facts with rescue statements everywhere that says "failed to read fact x" so it's at least easier to understand.

Not a bad idea about improving the error reporting, Ewoud could you turn that into a new RM issue and describe the expected behavior on several examples please? Like if uploaded facts does not contain fact xyz, he should see an error saying "fact xyz is not present". Ideally also specify where (notification drawer, log, API response, host status or something else)

I agree we should go ahead and drop support for legacy facts, I asked few users in our community and there was no concern with that. People seem to be upgrading their Puppet infrastructure (good). If they have older facters deployed, they probably also have older Foreman installed.

https://github.com/puppetlabs/facter/commit/58901c0101e8c8d7f1ec80bcb9356e61d7cadee3 did add a schema of facts. Its history can be used as a source of when a fact was introduced. That's the best way right now that I know to determine it.