Replaced FileNotFoundException with log instead (#199)

[LennardF1989]

Fixes #199.

• I added a logger on all applicable places in support of the reported issue.
• Updated the unit test to check for a useful log-attempt instead of throwing an exception.
• Tested the implemention using Smidge.Web both with and without AddSmidgeInMemory and manipulating some of the generated /sc-URLs:
  

Upon testing, it looks like /sb-URLs could use a similar treatment as it will throw an exception when the bundle-key or version is invalid, but I'd say that's something for another PR.

EDIT: PS. I removed a myget-reference that is no longer valid and prevents restoring packages.

[PhyxionNL]

I think this should be handled differently, now fileInfo is returned even when its useless (also in MemoryCacheFileSystem).

[LennardF1989]

Which is as intended, as other parts of the logic will check for .Exists, which is precisely what a FileInfo is for (a cheap wrapper to tell you if a file exists before attempting to open it). You also annotated the old code :)

Returning null here would mean a lot more code has to be touched and double-checked.

[PhyxionNL]

Which is as intended, as other parts of the logic will check for .Exists, which is precisely what a FileInfo is for (a cheap wrapper to tell you if a file exists before attempting to open it). You also annotated the old code :)

Returning null here would mean a lot more code has to be touched and double-checked.

It's already documented that GetRequiredFileInfo throws:

Smidge/src/Smidge.Core/ISmidgeFileSystem.cs

Line 24 in 994e945

/// Throws an exception if the file doesn't exist

So this would be breaking anyway.

Not all other paths check if the file exists (only SmidgeController does):

Smidge/src/Smidge.Core/FileProcessors/PreProcessManager.cs

Line 79 in 994e945

var sourceFile = new Lazy<IFileInfo>(() => _fileSystem.GetRequiredFileInfo(file), LazyThreadSafetyMode.None);

Smidge/src/Smidge.Core/FileProcessors/CssImportProcessor.cs

Line 50 in 994e945

var filePath = _fileSystem.GetRequiredFileInfo(path);

This would also apply to other people using the library, now it doesn't throw anymore and you need to handle it differently.

[LennardF1989]

Remark about breaking change is fair, the other ones are easily "fixed" by reintroducing the exception right after retrieving the IFileInfo. I will push that in a second.

The two locations you pointed out are literally the only 2 places where something is done with GetRequiredFileInfo. They are also only called in very specific situations: @import in CSS and during bundle creation, both of which cannot be directly manipulated by malicious users (only malicious developers :P). That said, bundle creation would also need some love as I pointed out, but for another PR.

[PhyxionNL]

Remark about breaking change is fair, the other ones are easily "fixed" by reintroducing the exception right after retrieving the IFileInfo.

Not everyone uses Smidge as-is, some have custom implementations around it. I don't mind fixing this at all, but let's try to get consistent behavior ☺️

I think it's fine not to throw anymore; then PreProcessManager can bail out, and CssImportProcessor can continue with the others.

Alternatively, SmidgeController could swallow the exception. This would be the least breaking change.
@Shazwazza what would you prefer?

[Shazwazza]

Yeah, i don't think we should change the behavior of this method since the meaning is in the name 'GetRequired...' ... which indicates that it will throw.

One way to be less breaking would be to have an optional overload of this like bool throwIfNotFound = true

It is a binary breaking change, but all folks nowadays build their projects before deploying so I don't think it would cause any issues.

Else I'm fine with try/catch/swallow where necessary

[Shazwazza]

@LennardF1989 just wanted to ping on this one?

[LennardF1989]

Honestly, I forgot that I did this xD I'll put it back on my list.