Skip to content

feat: Add timeouts to GitLab API calls

Elian Cordoba requested to merge ElianCordoba/gitlab-vscode-extension:add-request-timeout into main
  • 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 in code_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
Edited by 🤖 GitLab Bot 🤖

Merge request reports