fix: run `on_upstream_http_request` hook in federation source #440

YassinEldeeb · 2024-02-12T11:07:13Z

closes #432

github-actions · 2024-02-12T11:07:40Z

🚨 Rust Panic Audit: 87 Potential Panic Points Detected 🚨

Crate: `federation_query_planner`

📊 Total Usages: 51

🎁 unwrap usages: 30
🚨 panic usages: 3
🔎 expect usages: 8
🔢 array_index usages: 10

Crate: `engine`

📊 Total Usages: 11

🚨 panic usages: 3
🔎 expect usages: 1
🎁 unwrap usages: 7

Crate: `telemetry`

📊 Total Usages: 9

🎁 unwrap usages: 4
🔢 array_index usages: 5

Crate: `cloudflare_worker`

📊 Total Usages: 6

🚨 panic usages: 1
🎁 unwrap usages: 5

Crate: `tracing`

📊 Total Usages: 6

🎁 unwrap usages: 5
🔎 expect usages: 1

Crate: `conductor`

📊 Total Usages: 3

🔎 expect usages: 1
🚨 panic usages: 1
🎁 unwrap usages: 1

Crate: `config`

📊 Total Usages: 1

🚨 panic usages: 1

📌 Expected Annotations

Crate: `vrl`

📊 Total Expected Usages: 2

expand details

Reason: "if the provided VRL code in the config file can't compile, we have to exit."

Code: panic!("failed to compile vrl program");
Location: ./plugins/vrl/src/plugin.rs:122

Reason: "states is a non-user provided variable"

Code: .expect("can't merge states when states is an empty vector!")
Location: ./plugins/vrl/src/plugin.rs:139

Crate: `conductor`

📊 Total Expected Usages: 2

expand details

Reason: "we need to exit the process, if the logger can't be correctly set."

Code: tracing::subscriber::set_global_default(subscriber).expect("failed to set up tracing");
Location: ./bin/conductor/src/lib.rs:37

Reason: "we need to exit the process, if the provided configuration file is incorrect."

Code: panic!("Failed to initialize gateway: {:?}", e);
Location: ./bin/conductor/src/lib.rs:75

Crate: `napi`

📊 Total Expected Usages: 1

expand details

Reason: "we need this"

Code: panic!("Exited process!")
Location: ./libs/napi/src/lib.rs:18

Crate: `engine`

📊 Total Expected Usages: 5

expand details

Reason: "if we are unable to construct the endpoints and attach them onto the gateway's http server, we have to exit"

Code: Err(e) => panic!("failed to construct endpoint: {:?}", e),
Location: ./libs/engine/src/gateway.rs:147

Reason: "we can safely index here, it's inside a test with constant defined fixtures."

Code: ConductorGateway::execute(request, &gw.routes[0].route_data).await
Location: ./libs/engine/src/gateway.rs:179

Reason: "without a fetcher, there's no executor, without an executor, there's no gateway."

Code: panic!("Failed while initializing the executor's fetcher for Federation source");
Location: ./libs/engine/src/source/federation_source.rs:111

Reason: "without a fetcher, there's no executor, without an executor, there's no gateway."

Code: panic!("Failed while initializing the executor's fetcher for Federation source");
Location: ./libs/engine/src/source/federation_source.rs:142

Reason: "without a fetcher, there's no executor, without an executor, there's no gateway."

Code: panic!(
Location: ./libs/engine/src/source/graphql_source.rs:30

Crate: `common`

📊 Total Expected Usages: 1

expand details

Reason: "we're parsing a statically defined constant, we know it works ;)"

Code: .unwrap()
Location: ./libs/common/src/graphql.rs:23

Crate: `config`

📊 Total Expected Usages: 9

expand details

Reason: "👇"

Code: let raw_contents = read_to_string(file_path)
Location: ./libs/config/src/lib.rs:576

Reason: "👇"

Code: panic!("Failed to interpolate config file, please resolve the above errors");
Location: ./libs/config/src/lib.rs:608

Reason: "👇"

Code: parse_config_from_json(&config_string).expect("Failed to parse JSON config file")
Location: ./libs/config/src/lib.rs:615

Reason: "👇"

Code: parse_config_from_yaml(&config_string).expect("Failed to parse YAML config file")
Location: ./libs/config/src/lib.rs:619

Reason: "👇"

Code: _ => panic!("Unsupported config file extension"),
Location: ./libs/config/src/lib.rs:636

Reason: "👇"

Code: None => panic!("Config file has no extension"),
Location: ./libs/config/src/lib.rs:639

Reason: "part of development docgen CLI"

Code: .expect("Failed to serialize json schema for config file!");
Location: ./libs/config/src/generate-json-schema.rs:50

Reason: "part of development docgen CLI"

Code: .expect("Failed to write the json schema to the file system!");
Location: ./libs/config/src/generate-json-schema.rs:54

Reason: "statically defined regex pattern, we know it works ;)"

Code: .unwrap();
Location: ./libs/config/src/interpolate.rs:18

Crate: `cloudflare_worker`

📊 Total Expected Usages: 4

expand details

Reason: "it panics only if the header name is not valid, and we know it is."

Code: .unwrap()
Location: ./bin/cloudflare_worker/src/http_tracing.rs:20

Reason: "it panics only if the URL source is not valid, and it's already validated before."

Code: let url = req.url().unwrap();
Location: ./bin/cloudflare_worker/src/http_tracing.rs:23

Reason: "it only panics if we are not running in a CF context, should be safe."

Code: let cf_info = req.cf().unwrap();
Location: ./bin/cloudflare_worker/src/http_tracing.rs:27

Reason: "unwraps only in special cases where "data:text" is used."

Code: let http_host = url.host().unwrap().to_string();
Location: ./bin/cloudflare_worker/src/http_tracing.rs:36

Crate: `jwt_auth`

📊 Total Expected Usages: 1

expand details

Reason: "if initiating an http client fails, then we have to exit."

Code: let client = wasm_polyfills::create_http_client().build().unwrap();
Location: ./plugins/jwt_auth/src/jwks_provider.rs:49

github-actions · 2024-02-12T11:15:47Z

🐋 This PR was built and pushed to the following Docker images:

Docker Bake metadata

{
"conductor": {
  "containerimage.config.digest": "sha256:3f92bd76fa088714a0c5a2c3889f50dc7f4e6c3faf928513b27cb5860165c257",
  "containerimage.descriptor": {
    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "digest": "sha256:850bb01f800896373c8836bd92a62050ec9d43565b51e1b36fdf9ff2b8f399dc",
    "size": 902,
    "platform": {
      "architecture": "amd64",
      "os": "linux"
    }
  },
  "containerimage.digest": "sha256:850bb01f800896373c8836bd92a62050ec9d43565b51e1b36fdf9ff2b8f399dc",
  "image.name": "ghcr.io/the-guild-org/conductor/conductor:6aa851d3ba3d0cb4853dfab152c2d23605afc4c1"
}
}

github-actions · 2024-02-12T11:16:15Z

✅ Benchmark Results

     data_received..................: 13 MB   221 kB/s
     data_sent......................: 22 MB   363 kB/s
     http_req_blocked...............: min=1.04µs   avg=2.89µs   med=2.15µs   max=596.28µs p(95)=3.12µs   p(99)=12.76µs 
     http_req_connecting............: min=0s       avg=346ns    med=0s       max=526.22µs p(95)=0s       p(99)=0s      
     http_req_duration..............: min=311.65µs avg=408.99µs med=381.26µs max=21.43ms  p(95)=480.99µs p(99)=577.94µs
       { expected_response:true }...: min=311.65µs avg=408.99µs med=381.26µs max=21.43ms  p(95)=480.99µs p(99)=577.94µs
     ✓ { scenario:rps_1000 }........: min=311.65µs avg=408.99µs med=381.26µs max=21.43ms  p(95)=480.99µs p(99)=577.94µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 60001
     ✓ { scenario:rps_1000 }........: 0.00%   ✓ 0           ✗ 60001
     http_req_receiving.............: min=9.98µs   avg=25.75µs  med=25.36µs  max=847.68µs p(95)=33.47µs  p(99)=40.76µs 
     http_req_sending...............: min=7.03µs   avg=15.59µs  med=14.07µs  max=1.88ms   p(95)=26.86µs  p(99)=35.05µs 
     http_req_tls_handshaking.......: min=0s       avg=0s       med=0s       max=0s       p(95)=0s       p(99)=0s      
     http_req_waiting...............: min=278.19µs avg=367.64µs med=340.57µs max=21.38ms  p(95)=440.21µs p(99)=526.98µs
     http_reqs......................: 60001   1000.005233/s
     ✓ { scenario:rps_1000 }........: 60001   1000.005233/s
     iteration_duration.............: min=387.26µs avg=495.04µs med=466.24µs max=22ms     p(95)=569.54µs p(99)=723.56µs
     iterations.....................: 60001   1000.005233/s
     ✓ { scenario:rps_1000 }........: 60001   1000.005233/s
     valid_graphql_response.........: 100.00% ✓ 60001       ✗ 0    
     ✓ { scenario:rps_1000 }........: 100.00% ✓ 60001       ✗ 0    
     valid_http_code................: 100.00% ✓ 60001       ✗ 0    
     ✓ { scenario:rps_1000 }........: 100.00% ✓ 60001       ✗ 0    
     vus............................: 1       min=0         max=2  
     vus_max........................: 200     min=200       max=200

dotansimha

Please also make sure to call on_upstream_http_response on every response.

Also, some clippy notes :)

dotansimha

(see above)

dotansimha and others added 2 commits February 13, 2024 16:02

shared plugins manager

3d90971

add mutex to context

2537e67

YassinEldeeb force-pushed the run-missing-hook-in-fed-source branch from 67c7bb5 to 2537e67 Compare February 14, 2024 08:06

dotansimha approved these changes Feb 14, 2024

View reviewed changes

dotansimha requested changes Feb 14, 2024

View reviewed changes

fix clippy and run upstream response hook

6aa851d

dotansimha approved these changes Feb 14, 2024

View reviewed changes

dotansimha merged commit ccb9ff0 into master Feb 14, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: run `on_upstream_http_request` hook in federation source #440

fix: run `on_upstream_http_request` hook in federation source #440

YassinEldeeb commented Feb 12, 2024

github-actions bot commented Feb 12, 2024 •

edited

Loading

github-actions bot commented Feb 12, 2024 •

edited

Loading

github-actions bot commented Feb 12, 2024 •

edited

Loading

dotansimha left a comment

dotansimha left a comment

fix: run on_upstream_http_request hook in federation source #440

fix: run on_upstream_http_request hook in federation source #440

Conversation

YassinEldeeb commented Feb 12, 2024

github-actions bot commented Feb 12, 2024 • edited Loading

🚨 Rust Panic Audit: 87 Potential Panic Points Detected 🚨

Crate: federation_query_planner

Crate: engine

Crate: telemetry

Crate: cloudflare_worker

Crate: tracing

Crate: conductor

Crate: config

📌 Expected Annotations

Crate: vrl

Crate: conductor

Crate: napi

Crate: engine

Crate: common

Crate: config

Crate: cloudflare_worker

Crate: jwt_auth

github-actions bot commented Feb 12, 2024 • edited Loading

github-actions bot commented Feb 12, 2024 • edited Loading

✅ Benchmark Results

dotansimha left a comment

Choose a reason for hiding this comment

dotansimha left a comment

Choose a reason for hiding this comment

fix: run `on_upstream_http_request` hook in federation source #440

fix: run `on_upstream_http_request` hook in federation source #440

github-actions bot commented Feb 12, 2024 •

edited

Loading

Crate: `federation_query_planner`

Crate: `engine`

Crate: `telemetry`

Crate: `cloudflare_worker`

Crate: `tracing`

Crate: `conductor`

Crate: `config`

Crate: `vrl`

Crate: `conductor`

Crate: `napi`

Crate: `engine`

Crate: `common`

Crate: `config`

Crate: `cloudflare_worker`

Crate: `jwt_auth`

github-actions bot commented Feb 12, 2024 •

edited

Loading

github-actions bot commented Feb 12, 2024 •

edited

Loading