Skip to content

Conversation

@ArniStarkware
Copy link
Contributor

@ArniStarkware ArniStarkware commented Feb 8, 2026

Note

Low Risk
Test-only changes plus a small test utility builder setter; no production logic changes, with low risk beyond potential test flakiness from size assumptions.

Overview
Adds a parameterized test ensuring the HTTP server enforces static_config.max_request_body_size for both deprecated gateway and RPC add_tx endpoints, expecting 200 OK for allowed sizes and 413 PAYLOAD_TOO_LARGE when exceeded.

Extends HttpClientServerSetupBuilder test utilities with with_max_request_body_size() to easily override this config in tests.

Written by Cursor Bugbot for commit f091655. This will update automatically on new commits. Configure here.

@reviewable-StarkWare
Copy link

This change is Reviewable

@ArniStarkware ArniStarkware marked this pull request as ready for review February 8, 2026 14:11
Copy link
Contributor Author

ArniStarkware commented Feb 8, 2026

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch from 27b6c13 to 9709dea Compare February 8, 2026 14:20
@ArniStarkware
Copy link
Contributor Author

crates/apollo_http_server/src/http_server_test.rs line 430 at r1 (raw file):

    assert_eq!(
        response.status(),
        reqwest::StatusCode::PAYLOAD_TOO_LARGE,

@eitanm-starkware regarding the: RPC provider doesn't return an informative error when sncast declare fails.
This is the error message that is returned by our HTTP server.
Some message like this should return at some size...

Code quote:

        reqwest::StatusCode::PAYLOAD_TOO_LARGE,

@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch from 9709dea to 6afd26c Compare February 8, 2026 15:10
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_config branch from 4f57e64 to 2a03207 Compare February 8, 2026 15:10
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch from 6afd26c to 89af9bd Compare February 8, 2026 16:28
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_config branch 2 times, most recently from b4b1fb3 to 57eaeb6 Compare February 9, 2026 13:29
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch from 89af9bd to 5401e86 Compare February 9, 2026 13:29
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_config branch from 57eaeb6 to 8efb25f Compare February 9, 2026 13:31
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch 3 times, most recently from 2a88e73 to a9eb32e Compare February 10, 2026 08:17
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_config branch from 8efb25f to 3bac13d Compare February 10, 2026 08:17
Copy link
Contributor

@TzahiTaub TzahiTaub left a comment

Choose a reason for hiding this comment

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

@TzahiTaub reviewed 4 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware).


crates/apollo_http_server/src/http_server_test.rs line 430 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

@eitanm-starkware regarding the: RPC provider doesn't return an informative error when sncast declare fails.
This is the error message that is returned by our HTTP server.
Some message like this should return at some size...

@eitanm-starkware Should this be wrapped in a starknet error as this is what RPC providers always expect? Currently is a standard http error


crates/apollo_http_server/src/http_server_test.rs line 385 at r3 (raw file):

        mock_gateway_client.expect_add_tx().times(1).return_const(Ok(default_gateway_output()));
    }
    let http_client = HttpClientServerSetupBuilder::new(index)

Can we remove the index from the input?

Suggestion:

HttpClientServerSetupBuilder::new(unique_u16!())

@ArniStarkware
Copy link
Contributor Author

crates/apollo_http_server/src/http_server_test.rs line 430 at r1 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

@eitanm-starkware Should this be wrapped in a starknet error as this is what RPC providers always expect? Currently is a standard http error

It will be hard to implement and maintain a wrap of this error with a starknet error.

@ArniStarkware
Copy link
Contributor Author

crates/apollo_http_server/src/http_server_test.rs line 385 at r3 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

Can we remove the index from the input?

Your suggestion, as written, does not work. I tried it now.

thread 'http_server::http_server_test::request_body_size_limit_enforced::case_3_add_deprecated_gateway_tx_too_large' (684288) panicked at crates/apollo_http_server/src/test_utils.rs:171:17:
Failed spawning test http server

@ArniStarkware ArniStarkware changed the base branch from arni/gateway_http_server/max_request_body_size_config to graphite-base/12371 February 10, 2026 14:11
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch from a9eb32e to 108c567 Compare February 10, 2026 14:11
@ArniStarkware ArniStarkware changed the base branch from graphite-base/12371 to main-v0.14.1-committer February 10, 2026 14:11
@ArniStarkware ArniStarkware force-pushed the arni/gateway_http_server/max_request_body_size_test branch from 108c567 to f091655 Compare February 10, 2026 15:00
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.

3 participants