feat: Add timeouts to GitLab API calls
-
Please check this box if this contribution uses AI-generated content (including content generated by GitLab Duo features) as outlined in the GitLab DCO & CLA
Description
Related Issues
Resolves #1323 (closed)
How has this been tested?
I created 3 pairs of unit tests for the 3 main code paths that use fetch
. One without a timeout (called signal because of the web standard) and one with a very short timeout that ensures the test will fail. For this, I created a dummy server the send the request to that would experience a fake delay.
There are two implementation details I'm unsure about, the first one is the fact that I had to break the one test file per production file convention because of the presence of this line jest.mock('../../fetch_logged');
.
If I remove that my tests would work but not the existing one, and vice versa. I tried a couple of things like scoping the mock to only be on the needed describe
or trying to unmock or clear the mock but I wasn't able to do it. This most likely can be fixed I'm just not knowledgeable enough of jest.
The other thing is that before request errors would not throw an error but rather respond with ok = false
, and this would be caught by the handleFetchError
and re-throw with a different error kind (FetchError
). This is different with timeouts, they throw an AbortError
, meaning that the existing error patching code path won't be called. I didn't want to introduce too many changes but maybe a better approach would be to catch these timeout errors and create a fake response to pass through the handleFetchError
function, just to ensure consistency.
Finally, I had to add a couple of signal: AbortSignal....
to the test expected result because they were breaking now that every request has a default timeout value.
Screenshots (if appropriate)
Types of changes
Open questions
- Show I add a test for the
fetchCompletions
method incode_get_suggestions_provider.ts
? - Should I add the timeout to the
snowplow.ts
file? - I'm unsure about the location of the
REQUEST_TIMEOUT
-
Bug fix (non-breaking change which fixes an issue) -
New feature (non-breaking change which adds functionality) -
Breaking change (fix or feature that would cause existing functionality to change) -
Documentation -
Chore (Related to CI or Packaging to platforms) -
Test gap
Merge request reports
Activity
Hey @ElianCordoba!
Welcome to our community! We're excited to have you here, and can't wait to review this first MR with you!
Thank you for your contribution to GitLab. Please refer to the contribution documentation for an overview of the process.
Did you know about our community forks? Working from there will make your contribution process easier. Please check it out!
When you're ready for a first review, post
@gitlab-bot ready
. If you know a relevant reviewer(s) (for example, someone that was involved in a related issue), you can also assign them directly with@gitlab-bot ready @user1 @user2
.At any time, if you need help, feel free to post
@gitlab-bot help
or initiate a mentor session on Discord. Read more on how to get help.You can make AI-generated contributions to GitLab! If you use AI-generated content please check the box added to the top of your merge request description.
This message was generated automatically. You're welcome to improve it.
added 1st contribution Community contribution workflowin dev labels
assigned to @ElianCordoba
added linked-issue label
@gitlab-bot ready
added workflowready for review label and removed workflowin dev label
Hi Coach
@kerrizor
, this Community contribution is ready for review or needs your coaching.- Do you have capacity and domain expertise to review this? If not, find one or more reviewers and assign to them.
- If you've reviewed it, add the workflowin dev label if these changes need more work before the next review.
This message was generated automatically. You're welcome to improve it.
requested review from @kerrizor
- Resolved by Erran Carey
@erran since @viktomas is out, could you take over this Community contribution review?
mentioned in issue gitlab-org/quality/triage-reports#17603 (closed)
mentioned in issue #1323 (closed)
added workflowin review label and removed workflowready for review label
- Resolved by Tomas Vik
- Resolved by Elian Cordoba
- Resolved by Elian Cordoba
- Resolved by Elian Cordoba
- Resolved by Elian Cordoba