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

Fixes #112 and cleans up readers and stream #114

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Fixes #112 and cleans up readers and stream #114

wants to merge 6 commits into from

Conversation

danielwertheim
Copy link
Contributor

Fixes #112 by ensuring StreamReader and Stream is disposed as well as closing the JsonTextReader. The current implementation of Close in the JsonTextReader disposes the passed StreamReader but that should be the callers concerns in my opinion. Could do just a call to Close if that's preferred.

@danielwertheim
Copy link
Contributor Author

CI for one of the builds reports "An error occurred while generating the build script." Uncertain if that has anything to do with this PR. Don't see what I've messed up. I can see the former builds failing as I used newer C# constructs.

danielwertheim and others added 6 commits May 24, 2021 16:26
Reducing one try-catch construct as one has been introduced at higher level
and can deal with the lower-level case.

Also uses switch expression to get somewhat cleaner code.
There is no need to try and indentify the message if not an int.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JsonServerlessProtocol.TryParseMessage - missing cleanup
2 participants