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

Parsing response takes a very long time #371

Closed
BrianWhiting opened this issue Oct 24, 2024 · 6 comments
Closed

Parsing response takes a very long time #371

BrianWhiting opened this issue Oct 24, 2024 · 6 comments
Assignees
Labels
Enhancement New feature or request
Milestone

Comments

@BrianWhiting
Copy link

In my attempt to upgrade from 0.80.0 to 0.81.0, I noticed that calling client.CloudRecordings.GetRecordingsForUserAsync seemed to never return for certain users with a lot of recordings.

I was able to track down the issue to the new method ZoomNet.Internal.ParseZoomResponseAsync, specifically line 951 where it tries to do match a regular expression,

var matches = Regex.Match(responseContent, pattern, RegexOptions.IgnoreCase | RegexOptions.Singleline | RegexOptions.Compiled);
.

The regular expression takes longer and longer to execute as the number of recordings in the response grows. On my development machine, for 5 recordings, it takes less than 1 second, 10 recordings takes about 4 seconds to complete, and 15 recordings takes around 10 seconds.

@Jericho Jericho self-assigned this Oct 25, 2024
@Jericho Jericho added the Enhancement New feature or request label Oct 25, 2024
@Jericho Jericho added this to the 0.82.0 milestone Oct 25, 2024
@Jericho
Copy link
Owner

Jericho commented Oct 25, 2024

15 recordings takes around 10 seconds.

Yikes. That's not acceptable.

The reason this regex was added in 0.81.0 is to workaround a bug in the Zoom API: the response to some of our requests is malformed due to double quotes that are not properly escaped which prevents us from parsing the payload into a JSON document. I reported the issue to Zoom and I was told the following:

I have created a request internally to fix this issue (ZOOM-789295), and while we are trying to fix it, here is an example logic in js on how you can handle this issue at your end:

They proceeded to show me a JavaScript code sample with regex to detect and fix un-escaped double-quotes. The code you saw in the ParseZoomResponseAsync method is my C# interpretation of the JavaScript code they shared with me.

Either way, it's not an excuse to slow the parsing of the response to the extend that you have observed so I will figure out a way to improve the parsing speed while at the same time handle malformed JSON payload.

@Jericho
Copy link
Owner

Jericho commented Oct 25, 2024

I just uploaded a beta package to my personal NuGet feed (instructions). Do you think you can test it and verify that the solution I implemented solves the slow parsing issue?

@BrianWhiting
Copy link
Author

I have verified that the issue appears to be fixed with your 0.82.0-alpha.7 build. No apparent slowdown when the response contains 90 recordings.

@Jericho
Copy link
Owner

Jericho commented Oct 25, 2024

Excellent. Thank you for confirming. I'm waiting on a contributor to confirm #367. As soon as I hear back from him I will publish 0.82.0 to Nuget.

@Jericho
Copy link
Owner

Jericho commented Oct 26, 2024

Contributor confirmed functionality introduced in #367 therefore I'm proceeding with the release.

@Jericho Jericho closed this as completed Oct 26, 2024
@Jericho
Copy link
Owner

Jericho commented Oct 26, 2024

🎉 This issue has been resolved in version 0.82.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants