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

Fix 500 error for empty file uploads with Azure driver #296

Merged
merged 11 commits into from
Feb 11, 2020

Conversation

zone117x
Copy link
Member

Goal of PR

Fix #292
Fix #293

Fix regression where Azure driver was throwing a 5xx server error when a content-length: 0 header file POST was made.

Added integration tests for these cases -- found and fixed a bug with S3 driver returning undefined content-length in the same scenario.

Misc package upgrades.

Submission Checklist

  • Based on correct branch: feature submissions should be on develop, hotfixes should be on master

  • The code passes our eslint definitions, unit tests, and
    contains correct TypeFlow annotations.

  • Submission contains tests that cover any and all new functionality or code changes.

  • Submission documents any new features or endpoints, and describes how developers
    would be expected to interact with them.

  • Author has agreed to our contributor's agreement.

@codecov
Copy link

codecov bot commented Feb 10, 2020

Codecov Report

Merging #296 into develop will increase coverage by 7.93%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #296      +/-   ##
===========================================
+ Coverage    79.63%   87.56%   +7.93%     
===========================================
  Files           23       14       -9     
  Lines         1748     1472     -276     
  Branches       315      279      -36     
===========================================
- Hits          1392     1289     -103     
+ Misses         272      109     -163     
+ Partials        84       74      -10     
Impacted Files Coverage Δ
admin/src/config.ts
reader/src/config.ts
reader/src/index.ts
reader/src/http.ts
reader/src/server.ts
hub/src/index.ts
admin/src/index.ts
admin/src/server.ts
admin/src/http.ts
hub/src/server/utils.ts 84.88% <0.00%> (-7.87%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb47992...b307db7. Read the comment docs.

Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

LGTM!

@zone117x zone117x merged commit 68953e0 into develop Feb 11, 2020
@timstackblock timstackblock requested review from timstackblock and removed request for timstackblock February 18, 2020 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants