Skip to content

Improve error message when TestServer finishes with a pending request read#17164

Merged
JamesNK merged 2 commits into
masterfrom
jamesnk/testserver-pendingread
Dec 3, 2019
Merged

Improve error message when TestServer finishes with a pending request read#17164
JamesNK merged 2 commits into
masterfrom
jamesnk/testserver-pendingread

Conversation

@JamesNK

@JamesNK JamesNK commented Nov 16, 2019

Copy link
Copy Markdown
Member

Make it more obvious what the developer has done wrong.

@JamesNK JamesNK added this to the 5.0.0-preview1 milestone Nov 16, 2019
}
catch (Exception ex)
{
throw new InvalidOperationException("An error occurred when completing the request. Request delegate may have finished while there is a pending read of the request body.", ex);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's the inner exception type and message in this scenario?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

System.InvalidOperationException: Reading is already in progress.

I didn't want to completely throw away the original exception in case there is some scenario where an exception is thrown for a different reason.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure this is a substantial improvement. The exception type doesn't change, and the new message can't say with any confidence how to fix the problem.

Was there an issue associated with this PR? Was the IOE even reported to the client/test or was it swallowed internally?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Issue: grpc/grpc-dotnet#658

The current error isn't descriptive of the problem and how the developer could fix it.

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

Labels

area-hosting Includes Hosting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants