Skip to content

Add the name of the current type to the exception we throw in an unsupported case.#46183

Merged
CyrusNajmabadi merged 3 commits intodotnet:masterfrom
CyrusNajmabadi:exceptionInfo
Jul 22, 2020
Merged

Add the name of the current type to the exception we throw in an unsupported case.#46183
CyrusNajmabadi merged 3 commits intodotnet:masterfrom
CyrusNajmabadi:exceptionInfo

Conversation

@CyrusNajmabadi
Copy link
Contributor

We've received a customer report of this exception being thrown. However, it's in a scenario where many code actions are created. Adding specific information to the exception to help track down which may be at fault as we cannot repro this ourselves.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner July 21, 2020 18:38
@CyrusNajmabadi CyrusNajmabadi requested a review from a team July 21, 2020 18:51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

my supposition is that somehow the outer wrapping group item got triggered, causing us to eventaully call into GetChangedDocumentAsync below. That's the only thing i can think of that would be possible.

The change in this file is to make us resilient to someone calling that. The change below is to narrow down who is actually throwing if it's something else.

The two pieces of information i'm adding are to try to find out if that's the case.

Copy link
Member

Choose a reason for hiding this comment

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

Are we still going to hit GetChangedDocumentAsync when running into this scenario you described with this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we still going to hit GetChangedDocumentAsync when running into this scenario you described with this change?

So one of two thigns is true:

  1. Somehow, someone called GetOperations on a 'code action with nested actions' unexpectedly. If this it eh case, then the override of InnerInvoke here makes us resilient.
  2. Someone was calling into a CodeAction that isn't well behaved and is throwing in GetChangedDocument. If this is the case, then adding the "GetType().FullName" to the exception will tell us which code action is not well behaved.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@CyrusNajmabadi CyrusNajmabadi merged commit 41bcbef into dotnet:master Jul 22, 2020
@ghost ghost added this to the Next milestone Jul 22, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the exceptionInfo branch July 22, 2020 07:53
@RikkiGibson RikkiGibson modified the milestones: Next, 16.8.P2 Aug 11, 2020
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.

4 participants