Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

feat: include ip addr for diagnosing mmdb lookup failures #154

Merged
merged 1 commit into from
Jun 14, 2021
Merged

feat: include ip addr for diagnosing mmdb lookup failures #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/server/location.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,14 +340,14 @@ impl Location {
}

#[cfg(test)]
mod test {
pub mod test {
use super::*;
use crate::error::HandlerResult;
use std::collections::BTreeMap;

use actix_http::http::{HeaderName, HeaderValue};

const MMDB_LOC: &str = "mmdb/GeoLite2-City-Test.mmdb";
pub const MMDB_LOC: &str = "mmdb/GeoLite2-City-Test.mmdb";
const TEST_ADDR: &str = "216.160.83.56";

#[test]
Expand Down
7 changes: 6 additions & 1 deletion src/web/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ pub async fn get_tiles(
let addr = match ip_addr_str.parse() {
Ok(v) => v,
Err(e) => {
return Err(HandlerErrorKind::General(format!("Invalid remote IP {:?}", e)).into());
// Temporary: log the IP addr for debugging mmdb issues
return Err(HandlerErrorKind::General(format!(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since IPs are relatively sensitive information, should this be a configurable option that defaults to off? We could then turn it back off without having to cut a new release? (Or is there some other plan that isn't linked to from here?)

Copy link
Member Author

@pjenvey pjenvey Jun 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I can but I'm not intending for this to be used outside of the dev env (or worse case stage if we don't reproduce it there).

I'm working on a change for #148 that will land shortly where I'll be intending to revert his

"Invalid remote IP ({:?}) {:?}",
ip_addr_str, e
))
.into());
}
};
state
Expand Down
7 changes: 6 additions & 1 deletion src/web/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ use crate::{
build_app,
error::{HandlerError, HandlerResult},
metrics::Metrics,
server::{cache, location::Location, ServerState},
server::{
cache,
location::{test::MMDB_LOC, Location},
ServerState,
},
settings::{test_settings, Settings},
web::{dockerflow, handlers, middleware},
};
Expand All @@ -24,6 +28,7 @@ const UA: &str = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:72.0) Gecko/20100
fn get_test_settings() -> Settings {
let treq = test::TestRequest::with_uri("/").to_http_request();
Settings {
maxminddb_loc: Some(MMDB_LOC.to_owned()),
port: treq.uri().port_u16().unwrap_or(8080),
host: treq.uri().host().unwrap_or("localhost").to_owned(),
..test_settings()
Expand Down