Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be resilient to documents/projects with null-filepaths in our persistence layer. #19446

Merged
merged 5 commits into from
May 11, 2017

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented May 11, 2017

No description provided.

@CyrusNajmabadi
Copy link
Member Author

Tagging @dotnet/roslyn-ide

@CyrusNajmabadi
Copy link
Member Author

Customer scenario

TypeScript crashes.

Bugs this fixes:

Reported directly from TypeScript.

Workarounds, if any

None.

Risk

Low.

Performance impact

Low.

Is this a regression from a previous update?

Yes.

Root cause analysis:

Our switch to sqlite introduced a crash for TypeScript. This is because our persistence layer assumed that all docuemnts/project would have associated non-null filepaths with them. In some scenarios TypeScript does not supply these values.

Because we've never required that these values be non-null, we need to be resilient to that sort of data flowing through to the persistence layer.

How was the bug found?

Direct report from TypeScript.

@CyrusNajmabadi
Copy link
Member Author

Tagging @MattGertz

}

return solution;
}

private IPersistentStorage GetStorage(Solution solution)
protected IPersistentStorage GetStorage(Solution solution)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can use it from SQLitePersistentStorageTests.cs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants