search

dahall / TaskScheduler

Provides a .NET wrapper for the Windows Task Scheduler. It aggregates the multiple versions, provides an editor and allows for localization.
MIT License
1.21k stars 191 forks source link

Task.Folder can be null #871

Closed PhyxionNL closed 3 years ago

PhyxionNL commented 3 years ago

Describe the bug https://github.com/dahall/TaskScheduler/blob/fbcee2ea07b0c0ee1ca7e4adbc772836458ad2c6/TaskService/Task.cs#L1086

This is marked as NotNull, but this is not correct. If you have a task and delete the folder it's contained in then Folder will return null. See also: https://github.com/dahall/TaskScheduler/blob/fbcee2ea07b0c0ee1ca7e4adbc772836458ad2c6/TaskService/TaskService.cs#L663

To Reproduce Steps to reproduce the behavior:

  1. Create a task and keep an instance to it.
  2. Delete the task and its folder with Windows' task scheduler.
  3. Access Task.Folder.
dahall commented 3 years ago

If you have a task instance, and delete the folder to which it is assigned, my understanding is that all of the contained tasks and subfolders are also deleted. I believe the error you are seeing is that the Task should no longer be valid and not just the Parent property.

In your described scenario, is the task instance still available from the system? This would be similar to getting a FileInfo instance for a file and then deleting the file from the system. You would still have a valid class instance but it does not represent a physical system instance.

PhyxionNL commented 3 years ago

The Task is no longer valid as such, as it no longer exists, but Folder can return null then (which is incorrect with the attribute) . Alternatively, Folder could throw the original DirectoryNotFoundException. Throwing is probably better as some of the other properties also throw it, but would cause more things to be affected as GetFolder seems to be used in other places too.

dahall commented 3 years ago

I think, given this scenario that I've never really accounted for, that I should review key functions and properties and ensure that they all throw an exception when trying to act on a folder or task that no longer exists. Doesn't that seem the appropriate behavior?

PhyxionNL commented 3 years ago

I agree. Most properties of Task already throw a DirectoryNotFoundException if the folder is deleted. Some properties still work, such as the name, which is OK I guess.

dahall commented 3 years ago

I'm considering putting in a simple method that I put in before any property is fetched that would check with TaskService to see if the task still exists and throw an exception if it was removed. Is that appropriate or too intrusive?

dahall commented 3 years ago

For now, I check for null and throw a DirectoryNotFoundException exception if returned from GetFolder. After more investigation, and your feedback, I may still implement the more elaborate check for deleted tasks and folders.

PhyxionNL commented 3 years ago

I think that's a good enough change for now. I haven't tried all properties but the ones I'm using already throw a DirectoryNotFoundException.