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

upgrade axios-cookiejar-support to support axios 1.0 #209

Closed
huineng opened this issue Oct 5, 2022 · 11 comments
Closed

upgrade axios-cookiejar-support to support axios 1.0 #209

huineng opened this issue Oct 5, 2022 · 11 comments
Assignees

Comments

@huineng
Copy link
Member

huineng commented Oct 5, 2022

in your package json you are using axios-cookiejar-support ^1.0.0

this version is already at 4.0.3 https://github.com/IBM/node-sdk-core/blob/main/package.json

today axios itself upgraded to version 1 https://github.com/axios/axios/releases/tag/v1.0.0

and when using axios v1, node_sdk is no longer working

example of an error

Cannot find module 'axios/lib/defaults' from 'node_modules/axios-cookiejar-support/lib/index.js'

Require stack:
      node_modules/axios-cookiejar-support/lib/index.js
      node_modules/ibm-cloud-sdk-core/lib/request-wrapper.js
      node_modules/ibm-cloud-sdk-core/auth/token-managers/token-manager.js
      node_modules/ibm-cloud-sdk-core/auth/token-managers/jwt-token-manager.js
      node_modules/ibm-cloud-sdk-core/auth/token-managers/iam-request-based-token-manager.js
      node_modules/ibm-cloud-sdk-core/auth/token-managers/iam-token-manager.js
      node_modules/ibm-cloud-sdk-core/auth/token-managers/index.js
      node_modules/ibm-cloud-sdk-core/auth/authenticators/cloud-pak-for-data-authenticator.js
      node_modules/ibm-cloud-sdk-core/auth/authenticators/index.js
      node_modules/ibm-cloud-sdk-core/auth/index.js
      node_modules/ibm-cloud-sdk-core/lib/base-service.js
      node_modules/ibm-cloud-sdk-core/index.js
      node_modules/@ibm-cloud/cloudant/cloudant/v1.js
      node_modules/@ibm-cloud/cloudant/index.js

ps i might need to check with that library if it's also compatible with axios 1.0

thanks

is it possible to make the library compatible with axios 1.0 and the latest version of axios-cookiejar-support

thanks

@andyedwardsibm
Copy link

Hitting this as well. As a simple recreate...

$# npm install axios@1.0.0 ibm-cloud-sdk-core

$# npm ls axios
recreate@ /tmp/recreate
├── axios@1.0.0
└─┬ ibm-cloud-sdk-core@3.1.0
  ├─┬ axios-cookiejar-support@1.0.1
  │ └── axios@1.0.0 deduped
  ├── axios@0.26.1
  └─┬ retry-axios@2.6.0
    └── axios@1.0.0 deduped

$# node -e "const Authenticator = require('ibm-cloud-sdk-core'); console.log('All done, bye bye')"
node:internal/modules/cjs/loader:499
      throw e;
      ^

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/defaults' is not defined by "exports" in /tmp/recreate/node_modules/axios/package.json
    at new NodeError (node:internal/errors:387:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:439:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:718:3)
    at resolveExports (node:internal/modules/cjs/loader:493:36)
    at Module._findPath (node:internal/modules/cjs/loader:533:31)
    at Module._resolveFilename (node:internal/modules/cjs/loader:942:27)
    at Module._load (node:internal/modules/cjs/loader:804:27)
    at Module.require (node:internal/modules/cjs/loader:1022:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/tmp/recreate/node_modules/axios-cookiejar-support/lib/index.js:8:40) {
  code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'
}

Node.js v18.6.0

As @huineng says, this module pulls in axios-cookiejar-support@1.0.1, and that pulls in the latest version of axios, including major version bumps. If anything else pulls in axios@^1.0.0 then this breaks at the point of requiring ibm-cloud-sdk-core

@andyedwardsibm
Copy link

axios/axios#5000 might be relevant here

@dpopp07 dpopp07 self-assigned this Oct 5, 2022
@dpopp07
Copy link
Member

dpopp07 commented Oct 5, 2022

Thanks for the issue and the detail y'all - I am going to investigate this week

@ricellis
Copy link
Member

ricellis commented Oct 13, 2022

I'm not saying that we shouldn't update versions here, but I think maybe worth noting that automatic peerDependency installation is a re-introduced feature in NPM 7+. Whether a peerDependency is attempted to be installed depends on if it is explicitly depended on higher up in the tree. Since the ibm-cloud-sdk-core explicitly declares a dependency of "axios": "^0.26.1" (i.e. excludes 1.x versions of axios) AFAICT this problem only emerges where axios-cookiejar-support or retry-axios are sufficiently high in a consumer's dependency tree that their peerDependency of axios is also installed instead of using the 0.26.x version installed for the core (or, of course, as demonstrated above a higher version of axios is explicitly installed).

@dpopp07
Copy link
Member

dpopp07 commented Oct 13, 2022

@ricellis thanks for that input, that's helpful to remember and presents a solid workaround. Unfortunately, updating the core to use axios 1.0 is not going to be trivial and will almost certainly require a major release. I'm still running into testing issues with how the new version of axios interacts with the cookie jar and retry-axios packages. This is still in-progress but will likely be delayed by the issues

@braj1999
Copy link

@dpopp07 Is there any progress on this?

@jftanner
Copy link
Member

I encountered this issue after updating to Axios 1.1.3 from 0.26.1 in my app. The solution, for me, for now, was to downgrade to 0.27.2. But that's not ideal from a security perspective. If there's anything I can do to assist in patching this, please let me know.

@ricellis
Copy link
Member

ricellis commented Nov 14, 2022

@dpopp07 we hit a bug where server disconnects are ignored (i.e. don't cause errors) when accepting gzip encoded responses. The problem is resolved in axios 1.x so this upgrade has increased in importance for our SDK now. I've made an attempt in #213 but the CI doesn't seem to like it, all tests passing for me locally (Node 14, 16, 18) though.

@dpopp07
Copy link
Member

dpopp07 commented Nov 14, 2022

Sorry all for the lack of updates - I've been working on this in the background but juggling a lot of things at the moment. @ricellis I appreciate the work, I'll check out your PR now

@dpopp07
Copy link
Member

dpopp07 commented Dec 2, 2022

All - there is a beta version published now that incorporates axios v1.

It can be installed with the beta tag (npm i ibm-cloud-sdk-core@beta). If you would, try it out and follow up if you continue to have any issues. It will require updating your typescript version to v4 if you haven't already. If you're running tests, it will also require the latest version of jest.

@huineng
Copy link
Member Author

huineng commented Dec 22, 2022

So i guess this can now be closed as 4.0.0 has been published ?

@dpopp07 dpopp07 closed this as completed Dec 22, 2022
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

No branches or pull requests

6 participants