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

fix(TT-409): bubble up protocol errors to the client #25

Merged
merged 3 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
nathanosdev marked this conversation as resolved.
Show resolved Hide resolved
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(())
}
Loading