Skip to content

Fix build issues after upgrading reqwest from 0.12.0 to 0.12.14#226

Closed
douggynix wants to merge 4 commits intoTrueLayer:mainfrom
douggynix:feature/reqwest-upgrade-0.12.14
Closed

Fix build issues after upgrading reqwest from 0.12.0 to 0.12.14#226
douggynix wants to merge 4 commits intoTrueLayer:mainfrom
douggynix:feature/reqwest-upgrade-0.12.14

Conversation

@douggynix
Copy link

Reqwest 0.12.14 introduces a breaking change that removes a deprecated function.
Reqwest-middleware exposes a public function with the same prototype definition that has been removed:

    pub fn fetch_mode_no_cors(self) -> Self {
        RequestBuilder {
            inner: self.inner.fetch_mode_no_cors(),
            ..self
        }
    }

this function above has been removed from reqwest::RequestBuilder and was not advised to be used either.
So, I remove this function as well as suggested by reqwest

In addition to that, I also upgrade most of the remaining cargo dependencies for each modules. So, we were using really old packages. Migrating them now to latest should ease maintenance and upgrade for future releases.

@douggynix douggynix requested a review from a team as a code owner March 13, 2025 04:38
@douggynix
Copy link
Author

douggynix commented Mar 13, 2025

I am wondering why for most crates we decide to stick to specific minor versionsx.y.z instead of major sem version x.y
For example by the time I upgrade manually tokio 1.0.0 from 1.44.0, there is already a new updated version available which is 1.44.1. I would advise us to stick to x.y such that instead of targeting specific version of tokio we would target tokio = "1.44".
As a Library, sticking to major versions with sem ver x.y is more appropriate rather than specific ones.
Dependency update management would be seamless and easier. Let me know what do you think?
image

Copy link

@AS1100K AS1100K left a comment

Choose a reason for hiding this comment

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

The version update isn't required and will cause breaking change for the dependents. If a dependency is set to a version lets say 1.2.3, this means >=1.2.3, <2.0.0. This is specified in The Cargo Book.

So, since we are specifying the dependencies like 1.0.0, we are saying that just use any version from >=1.0.0, <2.0.0, this ensure that the dependent crate can use any version of our dependency. For example: You changed the version of anyhow from 1.0.0 to 1.0.97, this will make the dependent crate to use >=1.0.97 version of anyhow.

I hope you understand that updating dependencies is a huge breaking changes, and I strongly believe that the maintainers would also say the same once they review this PR.

@douggynix
Copy link
Author

I underystand the concern about broken changes and dependency update management.
In my case, I am using the latest reqwest in a project, and I am having build issues as there are deprecated function that has been removed which the library is still using?
there should be a mechanism to keep up the pace with library updates before hitting the wall with different versions of lib that can introduce in future breaking changes as well.

We can close this PR if it is against the maintainer Philosophy for this package.
I understand the concern though to maintain backward compatibility with those who are using it.
the method that has been removed should have been as well marked deprecated. So, if you're sticking with specific versions, either you migrate one by one the old versions.

we should be thinking of upgrading anyhow versions, as it is really old comparing to the latest one available.

@jdon
Copy link
Contributor

jdon commented Mar 24, 2025

Thanks for the PR @douggynix, we would prefer to not make a breaking changes, so I'm closing this in favour of #225

@jdon jdon closed this Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants