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

PROD Builds starting to fail since FailedToDeserializePathParams error #3190

Closed
gilgabo opened this issue Jan 22, 2025 · 21 comments
Closed

PROD Builds starting to fail since FailedToDeserializePathParams error #3190

gilgabo opened this issue Jan 22, 2025 · 21 comments

Comments

@gilgabo
Copy link

gilgabo commented Jan 22, 2025

Bug Report

Version

v0.8.1

Platform

Unix

Crates

axum

Description

#20 176.0 error[E0015]: cannot call non-const fn FailedToDeserializePathParams::status in constant functions
#20 176.0 --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-0.8.1/src/extract/rejection.rs:151:1
#20 176.0 |
#20 176.0 151 | / composite_rejection! {
#20 176.0 152 | | /// Rejection used for Path.
#20 176.0 153 | | ///
#20 176.0 154 | | /// Contains one variant for each way the Path extractor
#20 176.0 ... |
#20 176.0 159 | | }
#20 176.0 160 | | }
#20 176.0 | |^
#20 176.0 |
#20 176.0 = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
#20 176.0 = note: this error originates in the macro composite_rejection (in Nightly builds, run with -Z macro-backtrace for more info)
#20 176.0
#20 176.0 error[E0015]: cannot call non-const fn InvalidUtf8InPathParam::status in constant functions
#20 176.0 --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/axum-0.8.1/src/extract/rejection.rs:162:1
#20 176.0 |
#20 176.0 162 | / composite_rejection! {
#20 176.0 163 | | /// Rejection used for RawPathParams.
#20 176.0 164 | | ///
#20 176.0 165 | | /// Contains one variant for each way the RawPathParams extractor
#20 176.0 ... |
#20 176.0 170 | | }
#20 176.0 171 | | }
#20 176.0 | |
^
#20 176.0 |
#20 176.0 = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
#20 176.0 = note: this error originates in the macro composite_rejection (in Nightly builds, run with -Z macro-backtrace for more info)
#20 176.0

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 22, 2025

hmm, I guess the latest change in axum-core was technically a breaking change and is incompatible with axum 0.8.0 and 0.8.1. if you upgrade axum to 0.8.2 the issue should be resolved though.

@jplatte
Copy link
Member

jplatte commented Jan 22, 2025

Damn, what an awkward failure mode. I'd say to prevent this sort of thing from happening again, we should bite the bullet and copy-paste the macros to every individual crate. Then upgrading only axum-core w/o axum would have only added const there, which would not have been an error.

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 22, 2025

I'm not a fan of the duplication that that would bring. I'd say we need to be more careful with potential breaking changes in axum-core instead :)

@gilgabo gilgabo changed the title Builds starting to fail since FailedToDeserializePathParams error PROD Builds starting to fail since FailedToDeserializePathParams error Jan 22, 2025
@gilgabo
Copy link
Author

gilgabo commented Jan 22, 2025

Our company's PROD builds are still failing for this breaking change. Any workaround? Are you planning to solve at axum-core crate?

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 22, 2025

if you upgrade axum to 0.8.2 the issue should be resolved though

see above...

@gilgabo
Copy link
Author

gilgabo commented Jan 22, 2025

Yes, but upgrade the version means create hotfix release for all our projects 😟

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 22, 2025

I don't understand why you need to hotfix something.

  • if your project is a library then you are probably using e.g. axum = "0.8.0" syntax, which already allows 0.8.2 to be installed too.
  • if you project is an application then I recommend pinning all of your dependencies (e.g. axum = "=0.8.0") and putting the lockfile into git.

@gilgabo
Copy link
Author

gilgabo commented Jan 22, 2025

Yes, that's makes sense, but in a Productive environment you need to do a lot stuff just to change the version is not just modify the Cargo file (like when you are working in a POC or personal / small project).

For us it is a great effort, time and implies having downtimes of our products due to a breaking change that we are not introducing by us (it is a side effect of a change by Axum)

@jplatte
Copy link
Member

jplatte commented Jan 22, 2025

If upgrading is such a big deal, then how did you get into this situation in the first place? How did you upgrade axum-core without upgrading axum, and not notice that your app now failed to compile?

@gilgabo
Copy link
Author

gilgabo commented Jan 22, 2025

Pipelines were working properly in the afternoon without any change, everything was in Green state. At night we have triggered our validation pipelines again without introduce any change or update, just execute a build (which implies compile, deployment and other tasks) and they have started to fail with the error stack attached above.

For us, since we are not pushed any code or configuration change, It looks like a introduced side effect / change from axum since we are not updating any file in our projects

@oscarAppd2c
Copy link

I believe that introducing breaking changes, can have significant implications for production environments. While upgrading dependencies might seem straightforward in smaller projects or development environments, it often requires a more extensive process in larger, production-grade systems.
It might be worth considering ways to minimize such impacts in the future, whether through stricter versioning policies or improved communication of potential risks when introducing changes

@jplatte
Copy link
Member

jplatte commented Jan 22, 2025

Your pipeline must do some really weird stuff then, if it pulls in a new version of axum-core automatically, without updating other dependencies like axum.

I have yanked the new releases for now. If that doesn't fix your issue, there is nothing I can do for you. I'll look to release axum-core 0.5.2 / axum 0.8.3 / axum-extra 0.11.1 in the coming days, hopefully without any such issues.

@jplatte
Copy link
Member

jplatte commented Jan 22, 2025

I believe that introducing breaking changes, can have significant implications for production environments. While upgrading dependencies might seem straightforward in smaller projects or development environments, it often requires a more extensive process in larger, production-grade systems.

For sure. I'm not arguing with any of that.

It might be worth considering ways to minimize such impacts in the future, whether through stricter versioning policies or improved communication of potential risks when introducing changes

I'm happy to hear about any ideas towards this. Note that the breaking change being discussed here is extremely subtle: A (private) macro changed what it outputs such that existing uses of the macro in existing versions of other related crates stopped compiling because they were relying on other changes to those related crates that happened in the same PR. I don't think this can really be detected automatically - in particular cargo-semver-checks does not flag anything about axum-core v0.5.1.

@gilgabo
Copy link
Author

gilgabo commented Jan 22, 2025

Thank you very much @jplatte , everything is working perfectly again.

A couple of final comments / thoughts: our pipelines do nothing weird, just a fresh compilation, deployment and some validations, but nothing outside the community standards. For us, making changes is a big deal since our applications provide a large number of services to a large number of users, believing in the stability and reliability of Axum releases, so for us scheduling changes means ceasing to serve part or all of our consumers.

Perhaps in the future it will be very useful consider productive contexts like ours and be sure that the releases that are being generated are backward compatible with previous versions and do not generate breaking changes.

Thank you very much again for you time, effort and help!

@gilgabo gilgabo closed this as completed Jan 22, 2025
@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 22, 2025

our pipelines do nothing weird

if they pull in updated transitive dependencies, but not update top-level dependencies then IMHO they are doing something weird. once again, I recommend committing your Cargo.lock file in git, then you won't have this problem.

@jplatte
Copy link
Member

jplatte commented Jan 22, 2025

A couple of final comments / thoughts: our pipelines do nothing weird, just a fresh compilation, deployment and some validations, but nothing outside the community standards.

Well, if it's not your pipelines then maybe your Cargo.toml is pinning axum (by the way, unlike @Turbo87 I would not recommend this 😅) and you're not commiting the lockfile / including it in your pipelined builds (very bad idea).

Perhaps in the future it will be very useful consider productive contexts like ours and be sure that the releases that are being generated are backward compatible with previous versions and do not generate breaking changes.

Like I wrote above, this was an extremely subtle bug. Telling us we're not "consider[ing] productive contexts" (in an open source project that nobody is getting paid to maintain) is honestly insulting. We did our best and failed. It happens.

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 22, 2025

then maybe your Cargo.toml is pinning axum (by the way, unlike @Turbo87 I would not recommend this 😅)

well, I would only recommend it when using e.g. https://github.com/renovatebot/renovate to automatically get upgrade pull requests, which I would obviously also recommend 😉

@gilgabo
Copy link
Author

gilgabo commented Jan 22, 2025

A couple of final comments / thoughts: our pipelines do nothing weird, just a fresh compilation, deployment and some validations, but nothing outside the community standards.

Well, if it's not your pipelines then maybe your Cargo.toml is pinning axum (by the way, unlike @Turbo87 I would not recommend this 😅) and you're not commiting the lockfile / including it in your pipelined builds (very bad idea).

We are commiting the lock file for all our projects and that's the reason why we are thinking the issue was a side effect

Perhaps in the future it will be very useful consider productive contexts like ours and be sure that the releases that are being generated are backward compatible with previous versions and do not generate breaking changes.

Like I wrote above, this was an extremely subtle bug. Telling us we're not "consider[ing] productive contexts" (in an open source project that nobody is getting paid to maintain) is honestly insulting. We did our best and failed. It happens.

To be honest, I don't want to hurt anyone's feelings or make you feel insulted. We know that this is an Open Source project and your effort is important. All of us work on this industry and we are plenty aware that things could fail. Our pipelines are a good example 😆 . It was just a suggestion to keep in mind / improve the review process of backward compatibility before the releases.

@jplatte
Copy link
Member

jplatte commented Jan 22, 2025

We are commiting the lock file for all our projects and that's the reason why we are thinking the issue was a side effect

A side effect of what? Clearly your lock file is not getting fully respected by your build pipeline if a new crates.io release makes it into your builds without you updating said lockfile. I'd strongly suggest you look into what happened there 😉

@DarkFenX
Copy link

I'll look to release axum-core 0.5.2 / axum 0.8.3 / axum-extra 0.11.1 in the coming days, hopefully without any such issues.

What happened to this? I already put my (small) project into a situation where I removed some hacks in favor of builtin OptionalFromRequest for Json, and now I am stuck with yanked 0.8.2 / other dependencies as they are. Would be nice to have a non-yanked version with that functionality (minus changes which led to this issue).

@jplatte
Copy link
Member

jplatte commented Jan 31, 2025

Could take another week or two, sorry.

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

5 participants