fix: protect null frontmatter.tags in TaskFile.ts

[evolutioned]

Types of changes

Changes visible to users:

• New feature (prefix: feat - non-breaking change which adds functionality)
  • Issue/discussion:
  • WARNING: If the link is absent, the PR may be closed without review, due to the workload that such reviews usually require.
• Breaking change (prefix: feat!! or fix!! - fix or feature that would cause existing functionality to not work as expected)
  • Issue/discussion:
• Bug fix (prefix: fix - non-breaking change which fixes an issue)
• Documentation (prefix: docs - improvements to any documentation content for users)
• Sample vault (prefix: vault - improvements to the Tasks-Demo sample vault)
• Contributing Guidelines (prefix: contrib - any improvements to documentation content for contributors - see Contributing to Tasks)

Internal changes:

• Refactor (prefix: refactor - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
• Tests (prefix: test - additions and improvements to unit tests and the smoke tests)
• Infrastructure (prefix: chore - examples include GitHub Actions, issue templates)

Description

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

Checklist

• My code follows the code style of this project and passes yarn run lint.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• My change has adequate Unit Test coverage.

Terms

• My contribution follows this project's contributing guide
• I agree to follow this project's Code of Conduct

[claremacrae]

Hi @evolutioned, many thanks for contributing to Tasks.

Before we talk about the code change, we need to understand how to reproduce the bug. Then we can test the change, and work out how to add an automated test to ensure the problem never returns.

Can you give us a Markdown file that causes this problem please.

[evolutioned]

hi @claremacrae
Thank you for the push to reproduce the bug - I found one MD file that had invalid properties but once I opened it and Obsidian updated, it stopped producing the error in Obsidian Tasks - so I suspect it was an old file that was producing some issues in the Obsidian cache. It seems to be fixed now.

[claremacrae]

Thanks @evolutioned.

It is still worth your giving us the broken FrontMatter of your file so we can fix this for others.

Can you use the File Recovery core plugin, or Sync History or similar, to get the original content?

[evolutioned]

Hi @claremacrae
Happy to help. I'll just give context to when the issue arose.

Chronology

7.60 worked great.
7.7 upgrade seemed to break my tasks, resulting in "Loading Tasks..." in my daily notes query. Action: I didn't have time to investigate so just reverted to 7.60 via Github release download package, copying over the plugin. I left it for a while as I had other priorities, then I decided to try the latest update to 7.10 to see if the null reference excpetion setting on frontmatter.tags was fixed. I got the same result: "Loading Tasks..." on the query.
I took some time to rig up debugging to see if I could find the file that was failing and it seemed to be a file called Notes/DailyNotes/2021/May-2021/📅-Week-18-Review-(3-May-to-9-May)/3-May-2021-----Monday, (with the content below). This was originally an import from OneNote.
As soon as I opened that file in Obsidian, there must have been some Obsidian process that fixed the cache because when I turned the 7.10 plugin off and then on again, the query for tasks worked. Hence, why I close the PR.

So, I am unable to reproduce the conditions again to create that issue giving this anomaly. Is there anything here that gives a clue?

Notes/DailyNotes/2021/May-2021/📅-Week-18-Review-(3-May-to-9-May)/3-May-2021-----Monday

---

• Created: 2021-05-03 19:00:00 +1000
• Modified: 2021-05-04 21:28:54 +1000

---

• #amelia

[evolutioned]

[claremacrae]

Thanks for the info, but sorry, I was unclear.

As I understand, this PR is to fix a particular query that fails on a particular frontmatter content.

So please supply, as plain text in fenced code blocks:

• The query that failed
• The problem front matter

It needs to be fenced code blocks so that I can click on the Copy button and copy the content to write tests...

[claremacrae]

If you can't get back the original file, please try to recreate the situation.

[evolutioned]

I have this query on my Daily Note, so every day this is pulling back tasks. This is what failed with that message in the Debug Console.

not done
due before next week 
group by folder
sort by priority desc

The offending frontmatter was:

---
- Created: 2021-05-03 19:00:00 +1000
- Modified: 2021-05-04 21:28:54 +1000
---
- #amelia 

As I mentioned before, it was not occurring in 7.60 or below, then broke in .7.7 and when I tried 7.10.

As soon as I opened the offending file, the Obsidian Tasks plugin worked once turned off and back on again.

So I've tried for hours over the weekend to reproduce the conditions but have been unable to. My only guess is that the cache of this file's frontmatter was corrupt and by opening the file, Obsidian corrected it?

[claremacrae]

Great - thanks very much indeed.

[claremacrae]

Reopening as draft so I remember to look at this later.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[evolutioned]

Happy to help if I can.

[claremacrae]

I am unsure how to proceed with this.

I have tried hard with the code on main to make Obsidian trigger the error that this PR works around, and in all cases of invalid frontmatter that I have tried rawFrontMatter has been undefined, so the JSON.parse(JSON.stringify(rawFrontmatter)) call is never triggered.

This in turn makes it impossible to write a failing test that demonstrates the problem - and in turn, confirms that the PR behaves as intended.

I am not comfortable merging changes without tests.

I wonder whether Obsidian has become stricter in its parsing of formatter since the original problem happened. (I am on 1.7.1 Insider version)

[claremacrae]

Looking at the following lines:

obsidian-tasks/src/Scripting/TasksFile.ts

Lines 11 to 12 in e0bb9bd

	// Always make TasksFile.frontmatter.tags exist and be empty, even if no frontmatter present:
	private readonly _frontmatter = { tags: [] } as any;

line 21 of this code is wrong, as it always overwrites this._frontmatter, including if JSON.parse(JSON.stringify(rawFrontmatter)) returns a null or undefined value.

I suspect that this always-overwriting is the root cause of the issue you have found.

And you have worked around it by only updating this._frontmatter.tags if this._frontmatter is valid.

But if this._frontmatter is not valid, the TasksFile object is left in an invalid state.

As such, I don't think I can accept this PR as-is.

A fuller fix would not overwrite the this._frontmatter value if JSON.parse(JSON.stringify(rawFrontmatter)) returns invalid data.

/CC @ilandikov for comment too...

[claremacrae]

Example file that I have been experimenting with:

---
valid: stuff
invalid stuff
---

# pr_3082_null_tags_invalid_yaml

- [ ] #task Task in 'pr_3082_null_tags_invalid_yaml'

```tasks
path includes {{query.file.path}}
group by function JSON.stringify(task.file.property('tags'))
group by function JSON.stringify(task.file.frontmatter)
```

[evolutioned]

On my obsidian (1.6.7) running 7.10 tasks plugin, that example file parses correctly. I'm unable to reproduce that issue I had (screen shot before), so can only suspect that the original markdown file has been fixed by opening. Happy to reject to PR and accept the issue as no longer applying.