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

feat: Check for query limits before getting apex test results @W-8379096@ #92

Merged
merged 9 commits into from
Nov 19, 2020

Conversation

AnanyaJha
Copy link
Contributor

What does this PR do?

This PR adds in a check to ensure that we don't surpass the query limits and return a <h1>Bad Message 431</h1><pre>reason: Request Header Fields Too Large error. The query character limit should be 100,000 according to documentation, but from testing it appears to be closer to 12,000.

What issues does this PR fix or reference?

@W-8379096@

@AnanyaJha AnanyaJha requested a review from a team as a code owner November 10, 2020 20:12
@@ -27,6 +27,9 @@ import * as util from 'util';
import { nls } from '../i18n';
import { StreamingClient } from '../streaming';

// query char limit *should be* 100,000, but is closer to 12,000
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this based on testing we did or how did we come up with the 12,000 number ? Is it a know bug in the api that the 100,000 limit does not work ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's based on testing so far, but I've also reached out to the tooling api team to see if this is expected behavior

Choose a reason for hiding this comment

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

Since v48 it's 100,000, before it was 20,000.

Choose a reason for hiding this comment

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

The doc says "by default", not sure if that means it could be set lower for a particular org for whatever reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I originally had a check for the api version and used that to determine the query limit (100k vs 20k), but changed it to just 12,000 when the queries kept failing for me 😢 I'll update this based on what the tooling team says about the expected behavior

Copy link
Contributor Author

@AnanyaJha AnanyaJha Nov 18, 2020

Choose a reason for hiding this comment

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

The two limits we've been running into are REST API limits - the URI can't exceed 16,348 bytes & the URI + headers can't exceed 16,348 bytes from here. I've been able to consistently reproduce hitting the URI+headers limit with any query greater than 12,400 characters, so to be on the safe side I'd feel comfortable with keeping the limit at 12,400
A workaround for the limits is to use the soap api instead, so I looked into the equivalent of the metadata api invoke method you mentioned Bryan, but it doesn't look like anything like that exists for tooling api yet

@AnanyaJha AnanyaJha force-pushed the aj/queryLim branch 2 times, most recently from 1d4be2b to 3f28eb6 Compare November 18, 2020 22:35
Comment on lines 647 to 661
const record = {
Id: '7092M000000Vt94QAC',
Status: ApexTestQueueItemStatus.Completed,
ApexClassId: '01p2M00000O6tXZQAZ',
TestRunResultId: '05m2M000000TgYuQAK'
};

const records = [];
const queryIds = [];
let count = 700;
while (count > 0) {
records.push(record);
queryIds.push(record.Id);
count--;
}

Choose a reason for hiding this comment

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

Can 450-466 be reused here?

@AnanyaJha AnanyaJha merged commit 01cf724 into develop Nov 19, 2020
@AnanyaJha AnanyaJha deleted the aj/queryLim branch November 19, 2020 20:42
sfsholden pushed a commit that referenced this pull request Nov 20, 2020
sfsholden pushed a commit that referenced this pull request Nov 20, 2020
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.

3 participants