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

[dep] Upgrade axios from 0.21.4 to 1.6.0 #1108

Merged
merged 5 commits into from
Nov 10, 2023

Conversation

Drarig29
Copy link
Contributor

@Drarig29 Drarig29 commented Nov 9, 2023

What and why?

Closes #1096

Bumping axios to 1.6.0 came with 2 issues:

  • Cannot run app built with pkg 5.8.1 axios/axios#5866, fixed in a5c801a
  • Sending requests with FormData (which sends multipart/form-data payloads) was failing silently in our backend with only a warning.
    • For reference, the warning was Decoder has partially decoded field, which was probably caused by an incorrect boundary. Bumping form-data from 3.0.1 to 4.0.0 (compare changes) solved the issue: 41ce71b
    • This bug was caught in one of our e2e tests, for the junit upload command. I ensured this command also works with the standalone binary. I also tested with synthetics upload-application, which is another command using the form-data package.

How?

Bump axios and form-data, and add node_modules/axios/dist/node/axios.cjs to the pkg.assets to prevent the following error when running the standalone binary:

pkg/prelude/bootstrap.js:1872
      throw error;
      ^

Error: Cannot find module '/snapshot/my_app/node_modules/axios/dist/node/axios.cjs'
1) If you want to compile the package/file into executable, please pay attention to compilation warnings and specify a literal in 'require' call. 2) If you don't want to compile the package/file into executable and want to 'require' it from filesystem (likely plugin), specify an absolute path in 'require' call using process.cwd() or process.execPath.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)

This bug lead to a silent error,
with a `Decoder has partially decoded field` warning
in our backend. Maybe the boundary was incorrect.

This bug gets fixed with
form-data/form-data@v3.0.1...v4.0.0

I also tried with `axios@1.6.1`, but it didn't work.
@Drarig29 Drarig29 requested review from a team as code owners November 9, 2023 10:30
@Drarig29 Drarig29 requested review from juli1 and ericlaz November 9, 2023 10:30
@Drarig29 Drarig29 requested a review from jakepruitt November 9, 2023 11:02
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Nov 9, 2023

Datadog Report

Branch report: corentin.girard/upgrade-axios
Commit report: a1fb70d

datadog-ci-tests: 0 Failed, 0 New Flaky, 735 Passed, 0 Skipped, 1m 41.23s Total duration (3m 25.83s time saved)

@Drarig29 Drarig29 requested a review from a team November 9, 2023 16:22
@Drarig29 Drarig29 removed the request for review from ericlaz November 9, 2023 17:21
Copy link
Contributor

@etnbrd etnbrd left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Still curious about the standalone jest config.

{
tsconfig: 'tsconfig.json',
},
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

@Drarig29 Drarig29 Nov 10, 2023

Choose a reason for hiding this comment

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

For a moment I thought it was related to the first issue I had with axios (see this job):
image

Turns out it was really just a warning, but it's still a good thing to fix 😄

@Drarig29 Drarig29 removed the request for review from jakepruitt November 10, 2023 09:29
@Drarig29 Drarig29 merged commit 81bf230 into master Nov 10, 2023
10 checks passed
@Drarig29 Drarig29 deleted the corentin.girard/upgrade-axios branch November 10, 2023 09:32
Drarig29 added a commit that referenced this pull request Nov 10, 2023
* [dep] Upgrade axios from `0.21.4` to `1.6.0`

* Fix jest config

* Fix axios in standalone binary

* Fix bug when sending FormData

This bug lead to a silent error,
with a `Decoder has partially decoded field` warning
in our backend. Maybe the boundary was incorrect.

This bug gets fixed with
form-data/form-data@v3.0.1...v4.0.0

I also tried with `axios@1.6.1`, but it didn't work.

* Do not return early in `formatBackendErrors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

security vulnerability in axios SNYK-JS-AXIOS-6032459
6 participants