FileTokenCacheStore should clean up after failing to open the cache file
Opened this issue · 5 comments
We've seen exception reports in our telemetry that indicate that FileTokenCacheStore is throwing an IllegalStateException on line 117.
Code inspection shows that the reason is that the cache file couldn't be deserialized into an instance of MemoryTokenCacheStore. The comments indicate that once this happens, the cache file will not work again.
If this is the case, the file should be deleted and an instance of MemoryTokenCacheStore created so that the FileTokenCacheStore object can be successfully instantiated and the cache file can be re-created on the next write.
what version of ADAL were you using and was it repro-ing on the latest version?
I wasn't using any version of ADAL, these telemetry events come from partner teams who are using various versions.
However, the file in question was last updated with a formatting change in March, 2018, so the version shouldn't matter very much.
What's happening is that the attempt to read the file cache around line 91 throws an IOException, probably because of some file corruption. We know from this that the file exists but can't be successfully opened.
There is even a comment at line 114 describing that the FileTokenCacheStore is broken and will not work again, but it then just throws an exception rather than attempting to clean up.
Because the file isn't cleaned up, the exception happens every time the FileTokenCacheStore is instantiated and ADAL is essentially broken at this point.
Also, we don't have any theory or data about why the file was corrupted or the nature of that corruption. All we know is that the exception was thrown when the file existed. My point here is that it would make sense to clean up this file if you know that it will never successfully be opened again.
Thanks Joe. Do you happen to know which app or apps these are? I'm curious since the FileTokenCacheStore is not the default and I'd like to understand why they chose to use the FileTokenCacheStore as well as get a sense of how many people are impacted by this. Clearly when the impact is experience the only remediation is likely uninstall/re-install....
Sorry, this was a while ago, and our telemetry data isn't maintained for more than 30 days (usually less due to storage constraints). We stopped putting ADAL errors in our telemetry a few months ago, since they're generally not germane to MAM failures, and to help with the aforementioned storage constraints ;)