Skip to content

Commit

Permalink
fix(TT-409): bubble up protocol errors to the client (#25)
Browse files Browse the repository at this point in the history
* Bump the version of ic-agent to 0.38.2 so that it stops retrying 429s
indefinitely and bubbles up 429s
* Make the http gateway forward 429s from the node
* Add a test with a mock container
  • Loading branch information
raymondk authored Oct 22, 2024
1 parent e809448 commit 25722e8
Show file tree
Hide file tree
Showing 8 changed files with 838 additions and 32 deletions.
715 changes: 685 additions & 30 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ hyper-util = "0.1"

ic-cdk = "0.13"
ic-cdk-macros = "0.13"
ic-agent = "0.37"
ic-utils = "0.37"
ic-agent = "0.38.2"
ic-utils = "0.38.2"
candid = "0.10"
pocket-ic = "=3.0.0"
assert_matches = "1.5"
Expand Down
2 changes: 2 additions & 0 deletions packages/ic-http-gateway/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ ic-response-verification.workspace = true
[dev-dependencies]
assert_matches.workspace = true
pocket-ic.workspace = true
reqwest = "0.12.8"
testcontainers = "0.23.1"
tokio.workspace = true
8 changes: 8 additions & 0 deletions packages/ic-http-gateway/src/protocol/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,14 @@ fn handle_agent_error(error: &AgentError) -> CanisterResponse {
"Response size exceeds limit",
),

AgentError::HttpError(payload) => match StatusCode::from_u16(payload.status) {
Ok(status) => create_err_response(status, &format!("{:?}", payload)),
Err(_) => create_err_response(
StatusCode::INTERNAL_SERVER_ERROR,
&format!("Received invalid status code {:?}", payload),
),
},

// Handle all other errors
_ => create_err_response(
StatusCode::INTERNAL_SERVER_ERROR,
Expand Down
23 changes: 23 additions & 0 deletions packages/ic-http-gateway/test-container/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
FROM python:3.13.0-bookworm

# Prevents Python from buffering stdout and stderr
ENV PYTHONUNBUFFERED=1

# Set the working directory in the container
WORKDIR /app

# Move everything into the container
COPY . .

# Install dependencies
RUN pip install -r requirements.txt

# expose the server port
EXPOSE 8000

# a health check so we can wait for testcontainers
HEALTHCHECK --interval=1s --timeout=1s --start-period=1s --retries=3 \
CMD curl --fail http://localhost:8000/healthcheck || exit 1

# Run the flask server
CMD ["python", "app.py"]
18 changes: 18 additions & 0 deletions packages/ic-http-gateway/test-container/app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'''
a simple http server that returns 429s for all post requests
'''
from flask import Flask, request, jsonify

app = Flask(__name__)

@app.route('/healthcheck', methods=['GET'])
def healthcheck():
return "ok"

@app.route('/<path:any_path>', methods=['POST'])
def handle_post(any_path):
return "You're making too many requests", 429

if __name__ == '__main__':
app.run('0.0.0.0', port=8000)

7 changes: 7 additions & 0 deletions packages/ic-http-gateway/test-container/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
blinker==1.8.2
click==8.1.7
Flask==3.0.3
itsdangerous==2.2.0
Jinja2==3.1.4
MarkupSafe==3.0.1
Werkzeug==3.0.4
93 changes: 93 additions & 0 deletions packages/ic-http-gateway/tests/protocol_error_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
use http::{status::StatusCode, Request};
use ic_agent::{export::Principal, Agent};
use ic_http_gateway::{HttpGatewayClient, HttpGatewayRequestArgs};
use reqwest::Client;
use std::{env, error::Error, process::Command, str::FromStr};
use testcontainers::{
core::{IntoContainerPort, WaitFor},
runners::AsyncRunner,
GenericImage,
};

const IMAGE_NAME: &str = "ic-mock-busy-replica";
const IMAGE_TAG: &str = "latest";

fn build_gateway_image() -> Result<(), Box<dyn Error>> {
let cwd = env::var("CARGO_MANIFEST_DIR")?;

let output = Command::new("docker")
.current_dir(format!("{cwd}/test-container"))
.arg("build")
.arg("--file")
.arg("Dockerfile")
.arg("--force-rm")
.arg("--tag")
.arg(format!("{IMAGE_NAME}:{IMAGE_TAG}"))
.arg(".")
.output()?;

if !output.status.success() {
eprintln!("stderr: {}", String::from_utf8(output.stderr)?);
return Err("unable to build mock busy replica image.".into());
}

Ok(())
}

#[tokio::test]
async fn test_rate_limiting_error() -> Result<(), Box<dyn std::error::Error>> {
build_gateway_image()?;

// run the mock backend container
let container = GenericImage::new(IMAGE_NAME, IMAGE_TAG)
.with_exposed_port(8000.tcp())
.with_wait_for(WaitFor::healthcheck())
.start()
.await?;

// Retrieve the mapped port
let backend_port = container.get_host_port_ipv4(8000).await?;
let backend_host = container.get_host().await?.to_string();

// Check that the mock canister is up
let backend_base_url = format!("http://{}:{}", backend_host, backend_port);
let healthcheck_url = format!("{}/healthcheck", backend_base_url);
let response = Client::new().get(&healthcheck_url).send().await?;
assert_eq!(
response.status().as_u16(),
200,
"Expected to receive 200 from /healthcheck but received {}",
response.status().as_u16()
);

// Make a gateway
let agent = Agent::builder().with_url(backend_base_url).build().unwrap();
let http_gateway = HttpGatewayClient::builder()
.with_agent(agent)
.build()
.unwrap();

// Fake a `GET /example` request coming into the gateway
let canister_request = Request::builder()
.uri("/example")
.method("GET")
.body(Vec::<u8>::new())
.unwrap();

let gateway_response = http_gateway
.request(HttpGatewayRequestArgs {
canister_id: Principal::from_str("qoctq-giaaa-aaaaa-aaaea-cai")?,
canister_request,
})
.send()
.await;

assert_eq!(
gateway_response.canister_response.status(),
StatusCode::TOO_MANY_REQUESTS,
"Expected to receive a 429 from the node but received {}",
gateway_response.canister_response.status()
);

Ok(())
}

0 comments on commit 25722e8

Please sign in to comment.