feat: use workspace client [IDE-195] #24
Conversation
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
2 times, most recently
from
f443226
to
bcf0295
Compare
teodora-sandu
force-pushed
the
refactor/code-scanner-signature
branch
from
b20292c
to
d782ad9
Compare
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
13 times, most recently
from
5c44a69
to
611b5c0
Compare
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
3 times, most recently
from
527acbc
to
7a5cdff
Compare
| @@ -78,19 +79,14 @@ func (s *httpClient) Do(req *http.Request) (response *http.Response, err error) | |||
| return nil, err // no retries for errors | |||
| } | |||
|
|
|||
| err = s.checkResponseCode(response) | |||
| if err != nil { | |||
| if retryErrorCodes[response.StatusCode] { | |||
There was a problem hiding this comment.
Instead of throwing away the response (which may have useful information for the customer) and wrapping the status code in an error, we're going to return the original response and leave it up to the user of the HTTP client (deepcode vs workspace, for now) to decide what kind of error to return.
| @@ -99,7 +95,18 @@ func (s *httpClient) Do(req *http.Request) (response *http.Response, err error) | |||
|
|
|||
| func (s *httpClient) httpCall(req *http.Request) (*http.Response, error) { | |||
| log := s.logger.With().Str("method", "http.httpCall").Logger() | |||
|
|
|||
| // store the request body so that after retrying it can be read again | |||
| var copyReqBody io.ReadCloser | |||
There was a problem hiding this comment.
This is needed so that after every attempt we still have the request body. This code was refactored in #22 and before that change the body was built in every retry. I introduced a regression there so I decided to add a test as well to make sure this continues to work.
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
2 times, most recently
from
615014d
to
200b4be
Compare
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
from
200b4be
to
9500fda
Compare
| assert.Equal(t, 3, d.calls) | ||
| } | ||
|
|
||
| func TestSnykCodeBackendService_DoCall_shouldRetryWithARequestBody(t *testing.T) { |
There was a problem hiding this comment.
This is the test that verifies that the request body is still populated after a retry attempt.
| method string, | ||
| path string, | ||
| requestBody []byte, | ||
| ) ([]byte, error) { | ||
| log := s.logger.With().Str("method", "deepcode.Request").Logger() | ||
|
|
||
| host, err := s.Host() |
There was a problem hiding this comment.
Just a small code duplication improvement
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
from
9500fda
to
f85eb2a
Compare
|
|
||
| func (s *snykCodeClient) checkResponseCode(r *http.Response) error { | ||
| if r.StatusCode >= 200 && r.StatusCode <= 299 { | ||
| return nil | ||
| } | ||
| return fmt.Errorf("Unexpected response code: %s", r.Status) | ||
| } |
There was a problem hiding this comment.
Just a note: I think this is great but I wonder if we could potential get into a similar problem we got when querying the feature flag endpoint. That endpoint was returning a 403 to indicate that a flag was disables together with a comprehensive body message but making the handle of checkResponseCode a little more complicated.
There was a problem hiding this comment.
I think this is possible and for the workspace API I made sure that we don't get to this problem. The DeepCode API is a bit of an unknown area for me and this code is what we have in snyk-ls at the moment for this API so I would like to keep it as is: https://github.com/snyk/snyk-ls/blob/f3ca3f397a7bed8deee8c30d3916ffe2c810cb57/infrastructure/code/snyk_code_http_client.go#L434
teodora-sandu
force-pushed
the
feat/use-workspace-api
branch
from
f85eb2a
to
4fbe371
Compare
teodora-sandu
changed the title
Starts using the new Workspace API and adds a smoke test that uses a pre-prod org to run the end-to-end scanner in this repo.
This was tested by building
snyk-lsin snyk/snyk-ls#484 withmake build-debug, changing the custom endpoint tohttps://app.dev.snyk.io/apiand org toide-consistent-ignores-test.For some reason the feature flag check fails in dev too so I am having to comment out some of the code in that PR so that the feature flag is always true. I have raised a ticket to investigate: https://snyksec.atlassian.net/browse/IDE-243