build(deps): bump reqwest from `v0.11.18` to `v0.12.2` #4247

pixincreate · 2024-03-28T11:56:09Z

Type of Change

Description

This PR updates reqwest dependency version from 0.11.18 to 0.12.2 to address frequent disconnections and connection reset errors.

The major here is that we use actix_web along with reqwest. With reqwest releasing 0.12.0, http crate version was bumped from 0.x to 1.x while on actix_web v4.5.1 it is still on version v0.2.9. This version mismatch is what is handled here in this PR.

Also, in webhook.rs, it is not possible to directly convert headers that we get from reqwest to match with actix_web unless actix_web updates http version to 1.x. Also, since reqwest does not support HttpRequest explicitly, we cannot directly use that either which led to manual conversion. (Open for discussions on this)

reqwest changelog can be found here and notable breaking changes can be found here

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

Dependency update has been done in order to address frequent connection_reset errors.

Closes #4248

How did you test it?

Basic tests

Create { Merchant + API Key + MCA }
Update API Key
Make Payment
Refund

Postman tests

Once they succeeded, ran the postman collection for confirmation

Webhooks

Clickhouse

Set up clickhouse by following the guide here
http:localhost:8090

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible
I added a CHANGELOG entry if applicable

…1.18 to v0.12.2

…to v0.12.2

… v0.12.2

crates/analytics/src/clickhouse.rs

pixincreate · 2024-03-28T12:17:15Z

crates/router/src/core/webhooks.rs

+/*
+This is a temporary fix for converting http::HeaderMap from actix_web to reqwest
+Once actix_web upgrades the http version from v0.2.9 to 1.x, this can be removed
+*/
+fn convert_headers(
+    actix_headers: &actix_web::http::header::HeaderMap,
+) -> reqwest::header::HeaderMap {
+    let mut reqwest_headers = reqwest::header::HeaderMap::new();
+    for (name, value) in actix_headers.iter() {
+        if let Ok(name) = reqwest::header::HeaderName::from_str(name.as_str()) {
+            if let Ok(value) = reqwest::header::HeaderValue::from_bytes(value.as_bytes()) {
+                reqwest_headers.insert(name, value);
+            }
+        }
+    }
+    reqwest_headers
+}
+


we can have a discussion on this.
the pr to update http crate is open here: actix/actix-web#3208

crates/router/src/core/webhooks.rs

* 'main' of github.com:juspay/hyperswitch: refactor(core): removed the processing status for payment_method_status (#4213) docs(README): remove link to outdated early access form build(deps): bump `error-stack` from version `0.3.1` to `0.4.1` (#4188) chore(version): 2024.04.01.0 feat(mandates): allow off-session payments using `payment_method_id` (#4132) ci(CI-pr): determine modified crates more deterministically (#4233) chore(config): add billwerk base URL in deployment configs (#4243) feat(payment_method): API to list countries and currencies supported by a country and payment method type (#4126) chore(version): 2024.03.28.0 refactor(config): allow wildcard origin for development and Docker Compose setups (#4231) fix(core): Amount capturable remain same for `processing` status in capture (#4229) fix(euclid_wasm): checkout wasm metadata issue (#4198) fix(trustpay): [Trustpay] Add error code mapping '800.100.100' (#4224) feat(connector): [billwerk] add connector template code (#4123) fix(connectors): fix wallet token deserialization error (#4133) fix(log): adding span metadata to `tokio` spawned futures (#4118) chore(version): 2024.03.27.0 fix(connector): [CRYPTOPAY] Skip metadata serialization if none (#4205) fix(connector): [Trustpay] fix deserialization error for incoming webhook response for trustpay and add error code mapping '800.100.203' (#4199) fix(core): make eci in AuthenticationData optional (#4187)

This reverts commit 8f6f98b.

lsampras · 2024-04-01T12:33:20Z

crates/router/build.rs

is this increase in min stack size needed?

I'm not comfortable with us constantly bumping the number without attempting to address it.

Either revert this change, or if it is absolutely required, update the comment to reflect the updated number.

I started to get stack overflow on every run (stripe CI) resulting server shutdown until I bumped the min stack to 8 MiB which is why I bumped the number.

Since the bump has already been done here, the change is reverted.

SanchithHegde · 2024-04-01T12:54:27Z

crates/router/build.rs

I'm not comfortable with us constantly bumping the number without attempting to address it.

Either revert this change, or if it is absolutely required, update the comment to reflect the updated number.

crates/pm_auth/Cargo.toml

….0 instead of adding additional dependency

ArjunKarthik

connector changes LGTM

pixincreate · 2024-04-01T17:53:49Z

After today's discussions, it has been decided to better wait before getting this PR merged as actix_web has an open PR that updates http crate version to 1.x from 0.2.9 (which might take some time).

Once that is done, it will be easier for us to update the dependencies all together (actix_web, http and, reqwest) to their latest versions.

With that said, I'll be closing this PR to avoid any conflicts or confusion. I'll be raising yet another PR on behalf of this (just to keep the history clean) addressing the above mentioned changes.

This reverts commit ab97656, reversing changes made to b960e9f.

pixincreate added 8 commits March 26, 2024 14:29

chore(deps): [test_utils] bump reqwest from v0.11.18 to v0.12.2

5d3f098

chore(deps): [analytics] bump reqwest from v0.11.18 to v0.12.2

a23b597

chore(deps): [api_models] bump reqwest from v0.11.18 to v0.12.2

e9eff0d

chore(deps): [drainer] bump reqwest from v0.11.18 to v0.12.2

2331b8f

chore(deps): [common_utils & router] [unfixed] bump reqwest from v0.1…

ea973f4

…1.18 to v0.12.2

chore(deps): [pm_auth & router] [unfixed] bump reqwest from v0.11.18 …

82cabea

…to v0.12.2

chore(deps): [pm_auth & router] [fixed] bump reqwest from v0.11.18 to…

dd4afb4

… v0.12.2

chore(fmt-clippy): address rust fmt and clippy warning

560d2a5

pixincreate added A-dependencies Area: Dependencies S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor labels Mar 28, 2024

pixincreate requested review from lsampras, SanchithHegde, dracarys18, vspecky, Narayanbhat166 and Chethan-rao March 28, 2024 11:56

pixincreate self-assigned this Mar 28, 2024

pixincreate requested review from a team as code owners March 28, 2024 11:56

pixincreate commented Mar 28, 2024

View reviewed changes

crates/analytics/src/clickhouse.rs Outdated Show resolved Hide resolved

pixincreate commented Mar 28, 2024

View reviewed changes

pixincreate requested a review from ArjunKarthik March 28, 2024 12:34

pixincreate changed the title ~~chore(deps): [test_utils] bump reqwest from v0.11.18 to v0.12.2~~ chore(deps): bump reqwest from v0.11.18 to v0.12.2 Mar 28, 2024

pixincreate changed the title ~~chore(deps): bump reqwest from v0.11.18 to v0.12.2~~ build(deps): bump reqwest from v0.11.18 to v0.12.2 Mar 28, 2024

dracarys18 requested changes Apr 1, 2024

View reviewed changes

crates/router/src/core/webhooks.rs Outdated Show resolved Hide resolved

chore: address comments and stackoverflows

c19c65f

pixincreate requested a review from dracarys18 April 1, 2024 11:14

pixincreate added 3 commits April 1, 2024 17:01

refactor: log the error

8f6f98b

Revert "refactor: log the error"

25b30a6

This reverts commit 8f6f98b.

refactor: log the error

6ee0327

lsampras previously approved these changes Apr 1, 2024

View reviewed changes

SanchithHegde reviewed Apr 1, 2024

View reviewed changes

chore(fmt-clippy): revert rustminstack to 6mib and update http to 1.1…

b960e9f

….0 instead of adding additional dependency

pixincreate dismissed lsampras’s stale review via b960e9f April 1, 2024 13:41

pixincreate requested review from lsampras and SanchithHegde April 1, 2024 13:42

dracarys18 previously approved these changes Apr 1, 2024

View reviewed changes

ArjunKarthik previously approved these changes Apr 1, 2024

View reviewed changes

pixincreate closed this Apr 1, 2024

pixincreate deleted the update-reqwest branch April 1, 2024 17:53

pixincreate removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Apr 2, 2024

Narayanbhat166 restored the update-reqwest branch May 4, 2024 04:50

Narayanbhat166 reopened this May 4, 2024

Merge branch 'main' into update-reqwest

ab97656

Narayanbhat166 dismissed stale reviews from ArjunKarthik and dracarys18 via ab97656 May 4, 2024 05:07

Revert "Merge branch 'main' into update-reqwest"

59138b9

This reverts commit ab97656, reversing changes made to b960e9f.

pixincreate requested review from a team as code owners May 4, 2024 05:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build(deps): bump reqwest from `v0.11.18` to `v0.12.2` #4247

build(deps): bump reqwest from `v0.11.18` to `v0.12.2` #4247

pixincreate commented Mar 28, 2024 •

edited

pixincreate Mar 28, 2024 •

edited

lsampras Apr 1, 2024 •

edited

SanchithHegde Apr 1, 2024

pixincreate Apr 1, 2024 •

edited

SanchithHegde Apr 1, 2024

ArjunKarthik left a comment

pixincreate commented Apr 1, 2024

build(deps): bump reqwest from v0.11.18 to v0.12.2 #4247

Are you sure you want to change the base?

build(deps): bump reqwest from v0.11.18 to v0.12.2 #4247

Conversation

pixincreate commented Mar 28, 2024 • edited

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Basic tests

Postman tests

Webhooks

Clickhouse

Checklist

pixincreate Mar 28, 2024 • edited

Choose a reason for hiding this comment

lsampras Apr 1, 2024 • edited

Choose a reason for hiding this comment

SanchithHegde Apr 1, 2024

Choose a reason for hiding this comment

pixincreate Apr 1, 2024 • edited

Choose a reason for hiding this comment

SanchithHegde Apr 1, 2024

Choose a reason for hiding this comment

ArjunKarthik left a comment

Choose a reason for hiding this comment

pixincreate commented Apr 1, 2024

build(deps): bump reqwest from `v0.11.18` to `v0.12.2` #4247

build(deps): bump reqwest from `v0.11.18` to `v0.12.2` #4247

pixincreate commented Mar 28, 2024 •

edited

pixincreate Mar 28, 2024 •

edited

lsampras Apr 1, 2024 •

edited

pixincreate Apr 1, 2024 •

edited