Skip to content

Commit 1367935

Browse files
jenae-janzenjenae-janzen
committed
return permanent error + work on tests
1 parent 2192c51 commit 1367935

File tree

2 files changed

+89
-8
lines changed

2 files changed

+89
-8
lines changed

‎go/porcelain/deploy.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,6 @@ func (n *Netlify) WaitUntilDeployLive(ctx context.Context, d *models.Deploy) (*m
406406

407407
func (n *Netlify) uploadFiles(ctx context.Context, d *models.Deploy, files *deployFiles, observer DeployObserver, t uploadType, timeout time.Duration, skipRetry bool) error {
408408
sharedErr := &uploadError{err: nil, mutex: &sync.Mutex{}}
409-
permanentErr := &backoff.PermanentError{Err: nil}
410409
sem := make(chan int, n.uploadLimit)
411410
wg := &sync.WaitGroup{}
412411

@@ -435,7 +434,7 @@ func (n *Netlify) uploadFiles(ctx context.Context, d *models.Deploy, files *depl
435434
select {
436435
case sem <- 1:
437436
wg.Add(1)
438-
go n.uploadFile(ctx, d, file, observer, t, timeout, wg, sem, sharedErr, permanentErr, skipRetry)
437+
go n.uploadFile(ctx, d, file, observer, t, timeout, wg, sem, sharedErr, skipRetry)
439438
case <-ctx.Done():
440439
log.Info("Context terminated, aborting file upload")
441440
return errors.Wrap(ctx.Err(), "aborted file upload early")
@@ -455,7 +454,7 @@ func (n *Netlify) uploadFiles(ctx context.Context, d *models.Deploy, files *depl
455454
return sharedErr.err
456455
}
457456

458-
func (n *Netlify) uploadFile(ctx context.Context, d *models.Deploy, f *FileBundle, c DeployObserver, t uploadType, timeout time.Duration, wg *sync.WaitGroup, sem chan int, sharedErr *uploadError, permanentErr *backoff.PermanentError, skipRetry bool) {
457+
func (n *Netlify) uploadFile(ctx context.Context, d *models.Deploy, f *FileBundle, c DeployObserver, t uploadType, timeout time.Duration, wg *sync.WaitGroup, sem chan int, sharedErr *uploadError, skipRetry bool) {
459458
defer func() {
460459
wg.Done()
461460
<-sem
@@ -542,6 +541,14 @@ func (n *Netlify) uploadFile(ctx context.Context, d *models.Deploy, f *FileBundl
542541
context.GetLogger(ctx).WithError(operationError).Errorf("Failed to upload file %v", f.Name)
543542
apiErr, ok := operationError.(apierrors.Error)
544543

544+
// In testing, we repeatedly get to this line, see "Failed to upload file", but no matter
545+
// what we try, ok is always `false`, and the code we're trying to test gets skipped.
546+
// There isn't a test case for the original code (the 401 error)
547+
548+
// The operation error we're receiving is "(*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface"
549+
// which seems like a problem with how we're stubbing the response body from the server in the test,
550+
// but it's similar to how it's stubbed in
551+
545552
if ok {
546553
if apiErr.Code() == 401 {
547554
sharedErr.mutex.Lock()
@@ -550,7 +557,7 @@ func (n *Netlify) uploadFile(ctx context.Context, d *models.Deploy, f *FileBundl
550557
}
551558

552559
if skipRetry && (apiErr.Code() == 400 || apiErr.Code() == 422) {
553-
operationError = permanentErr
560+
operationError = backoff.Permanent(operationError)
554561
}
555562
}
556563
}

‎go/porcelain/deploy_test.go

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,19 +320,91 @@ func TestUploadFiles_Errors(t *testing.T) {
320320
err = client.uploadFiles(ctx, d, files, nil, fileUpload, time.Minute, false)
321321
require.Equal(t, err.Error(), "[PUT /deploys/{deploy_id}/files/{path}][500] uploadDeployFile default &{Code:0 Message:}")
322322
}
323-
func TestUploadFiles400Errors(t *testing.T) {
323+
324+
func TestUploadFiles400Error_SkipsRetry(t *testing.T) {
325+
attempts := 0
324326
ctx := gocontext.Background()
325327

326328
server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
329+
defer func() {
330+
attempts++
331+
}()
332+
327333
rw.WriteHeader(http.StatusUnprocessableEntity)
334+
rw.Header().Set("Content-Type", "application/json; charset=utf-8")
335+
rw.Write([]byte(`{"message": "Unprocessable Entity", "code": 422 }`))
328336
}))
329337
defer server.Close()
330338

339+
// // File upload:
340+
// hu, _ := url.Parse(server.URL)
341+
// tr := apiClient.NewWithClient(hu.Host, "/api/v1", []string{"http"}, http.DefaultClient)
342+
// client := NewRetryable(tr, strfmt.Default, 1)
343+
// client.uploadLimit = 1
344+
// ctx = context.WithAuthInfo(ctx, apiClient.BearerToken("token"))
345+
346+
// // Create some files to deploy
347+
// dir, err := ioutil.TempDir("", "deploy")
348+
// require.NoError(t, err)
349+
// defer os.RemoveAll(dir)
350+
// require.NoError(t, ioutil.WriteFile(filepath.Join(dir, "foo.html"), []byte("Hello"), 0644))
351+
352+
// files, err := walk(dir, nil, false, false)
353+
// require.NoError(t, err)
354+
// d := &models.Deploy{}
355+
// for _, bundle := range files.Files {
356+
// d.Required = append(d.Required, bundle.Sum)
357+
// }
358+
// // Set SkipRetry to true
359+
// err = client.uploadFiles(ctx, d, files, nil, fileUpload, time.Minute, true)
360+
361+
// // Function upload:
331362
hu, _ := url.Parse(server.URL)
332363
tr := apiClient.NewWithClient(hu.Host, "/api/v1", []string{"http"}, http.DefaultClient)
333364
client := NewRetryable(tr, strfmt.Default, 1)
334365
client.uploadLimit = 1
335-
ctx = context.WithAuthInfo(ctx, apiClient.BearerToken("bad"))
366+
apiCtx := context.WithAuthInfo(ctx, apiClient.BearerToken("token"))
367+
368+
dir, err := ioutil.TempDir("", "deploy")
369+
functionsPath := filepath.Join(dir, ".netlify", "functions")
370+
os.MkdirAll(functionsPath, os.ModePerm)
371+
require.NoError(t, err)
372+
defer os.RemoveAll(dir)
373+
require.NoError(t, ioutil.WriteFile(filepath.Join(functionsPath, "foo.js"), []byte("module.exports = () => {}"), 0644))
374+
375+
files, _, _, err := bundle(ctx, functionsPath, mockObserver{})
376+
require.NoError(t, err)
377+
d := &models.Deploy{}
378+
for _, bundle := range files.Files {
379+
d.RequiredFunctions = append(d.RequiredFunctions, bundle.Sum)
380+
}
381+
// Set SkipRetry to true
382+
require.NoError(t, client.uploadFiles(apiCtx, d, files, nil, functionUpload, time.Minute, true))
383+
require.Equal(t, err, "[PUT /deploys/{deploy_id}/files/{path}][422] uploadDeployFile default &{Code:422 Message: Unprocessable Entity}")
384+
require.Equal(t, attempts, 1)
385+
}
386+
387+
func TestUploadFiles400Error_NoSkipRetry(t *testing.T) {
388+
attempts := 0
389+
ctx := gocontext.Background()
390+
391+
server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) {
392+
defer func() {
393+
attempts++
394+
}()
395+
396+
rw.WriteHeader(http.StatusUnprocessableEntity)
397+
rw.Header().Set("Content-Type", "application/json; charset=utf-8")
398+
rw.Write([]byte(`{"message": "Unprocessable Entity", "code": 422 }`))
399+
return
400+
}))
401+
defer server.Close()
402+
403+
hu, _ := url.Parse(server.URL)
404+
tr := apiClient.NewWithClient(hu.Host, "/api/v1", []string{"http"}, http.DefaultClient)
405+
client := NewRetryable(tr, strfmt.Default, 1)
406+
client.uploadLimit = 1
407+
ctx = context.WithAuthInfo(ctx, apiClient.BearerToken("token"))
336408

337409
// Create some files to deploy
338410
dir, err := ioutil.TempDir("", "deploy")
@@ -346,8 +418,10 @@ func TestUploadFiles400Errors(t *testing.T) {
346418
for _, bundle := range files.Files {
347419
d.Required = append(d.Required, bundle.Sum)
348420
}
349-
err = client.uploadFiles(ctx, d, files, nil, fileUpload, time.Minute, true)
350-
require.Equal(t, err.Error(), "[PUT /deploys/{deploy_id}/files/{path}][401] uploadDeployFile default &{Code:401 Message: Unauthorized}")
421+
// Set SkipRetry to false
422+
err = client.uploadFiles(ctx, d, files, nil, fileUpload, time.Minute, false)
423+
// require.Equal(t, err, "[PUT /deploys/{deploy_id}/files/{path}][422] uploadDeployFile default &{Code:422 Message: Unprocessable Entity}")
424+
require.Equal(t, attempts, 12)
351425
}
352426

353427
func TestUploadFiles_SkipEqualFiles(t *testing.T) {

0 commit comments

Comments
 (0)