Log text of relevant file after an exception

[ghost]

Suggestion from #25790 (comment)

[sheetalkamat]

Change looks good but we should confirm with @DanielRosenwasser to see if its it meets user's consent criteria to have text of the file. Not sure if vs uses this stack in some way either. adding @amcasey to see if its ok.

[amcasey]

I don't believe there are GDPR concerns around logging customer content to a file that the customer controls. I'd be more concerned if we were including it in telemetry (which we're not, correct?).

When we request the log from customers, it's important to make sure they understand that their content will be included and to delete it promptly once the investigation is resolved.

[sheetalkamat]

@amcasey do we not log these in some kind of dumps of failed requests? Like issues #20629 ?

[amcasey]

@sheetalkamat We collect error messages (including stack traces). Is this modifying those? I thought it was just supplementing the log.

[sheetalkamat]

It is adding text of the file to the error messages. Eg. if #20629 it will include stack trace + text of file if it was coming from some kind of file request. (completion in the file eg.)

[sheetalkamat]

May be we should log the text only if LogLevel.verbose ?

[amcasey]

@sheetalkamat I'm still missing something. It looks like only the error message in the log is modified - not the one sent in the message response.

[ghost]

@amcasey This PR doesn't modify err.stack, it just logs something in addition to that.

Note that tsserver.log contains info about every command that happened, so it basically already contains every keypress you made, as well as the contents of files open in the editor.

[ghost]

@sheetalkamat @amcasey @DanielRosenwasser good to merge?

[amcasey]

@Andy-MS My understanding matches yours - this change has no effect on telemetry. Unless @sheetalkamat still has concerns, I think this is fine from a GDPR perspective.