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

Bring log and error message style in to line with rust guidelines #721

Merged
merged 4 commits into from
Mar 22, 2022
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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@ crates/*/Cargo.lock
.idea
*.iml
.vscode

# Docs
/docs/node_modules/
/docs/.cache/
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

-->


# [v0.1.0-preview.1] (unreleased) - 2022-mm-dd
## ❗ BREAKING ❗
## 🚀 Features
## 🐛 Fixes
- **Log and error message formatting** ([PR #721](https://github.com/apollographql/router/pull/721))

Logs and error messages in Rust begin with lower case and do not have trailing punctuation. The codebase is now consistent with this scheme.
## 🛠 Maintenance
## 📚 Documentation

# [v0.1.0-preview.0] - 2022-03-22

## 🎉 **The Apollo Router has graduated to its Preview phase!** 🎉
Expand All @@ -29,6 +40,7 @@ For more information on what's expected at this stage, please see our [release s
## 🐛 Fixes

- **Header propagation by `name` only fixed** ([PR #709](https://github.com/apollographql/router/pull/709))

Previously `rename` and `default` values were required (even though they were correctly not flagged as required in the json schema).
The following will now work:
```yaml
Expand Down
22 changes: 11 additions & 11 deletions apollo-router-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,37 @@ use typed_builder::TypedBuilder;
#[serde(tag = "type")]
#[ignore_extra_doc_attributes]
pub enum FetchError {
/// Query references unknown service '{service}'.
/// query references unknown service '{service}'
ValidationUnknownServiceError {
/// The service that was unknown.
service: String,
},

/// Invalid type for variable: '{name}'
/// invalid type for variable: '{name}'
ValidationInvalidTypeVariable {
/// Name of the variable.
name: String,
},

/// Query could not be planned: {reason}
/// query could not be planned: {reason}
ValidationPlanningError {
/// The failure reason.
reason: String,
},

/// Response was malformed: {reason}
/// response was malformed: {reason}
MalformedResponse {
/// The reason the serialization failed.
reason: String,
},

/// Service '{service}' returned no response.
/// service '{service}' returned no response.
SubrequestNoResponse {
/// The service that returned no response.
service: String,
},

/// Service '{service}' response was malformed: {reason}
/// service '{service}' response was malformed: {reason}
SubrequestMalformedResponse {
/// The service that responded with the malformed response.
service: String,
Expand All @@ -56,15 +56,15 @@ pub enum FetchError {
reason: String,
},

/// Service '{service}' returned a PATCH response which was not expected.
/// service '{service}' returned a PATCH response which was not expected
SubrequestUnexpectedPatchResponse {
/// The service that returned the PATCH response.
service: String,
},

/// HTTP fetch failed from '{service}': {reason}
///
/// Note that this relates to a transport error and not a GraphQL error.
/// note that this relates to a transport error and not a GraphQL error
SubrequestHttpError {
/// The service failed.
service: String,
Expand All @@ -73,16 +73,16 @@ pub enum FetchError {
reason: String,
},

/// Subquery requires field '{field}' but it was not found in the current response.
/// subquery requires field '{field}' but it was not found in the current response
ExecutionFieldNotFound {
/// The field that is not found.
field: String,
},

/// Invalid content: {reason}
/// invalid content: {reason}
ExecutionInvalidContent { reason: String },

/// Could not find path: {reason}
/// could not find path: {reason}
ExecutionPathNotFound { reason: String },
}

Expand Down
4 changes: 2 additions & 2 deletions apollo-router-core/src/query_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl CacheResolver<String, Option<Arc<Query>>> for QueryCacheResolver {
// Silently ignore cancelled tasks (never happen for blocking tasks).
Err(err) if err.is_cancelled() => None,
Err(err) => {
failfast_debug!("Parsing query task failed: {}", err);
failfast_debug!("parsing query task failed: {}", err);
None
}
};
Expand All @@ -47,7 +47,7 @@ impl QueryCache {
match self.cm.get(key).await {
Ok(v) => v,
Err(err) => {
failfast_debug!("Parsing query task failed: {}", err);
failfast_debug!("parsing query task failed: {}", err);
None
}
}
Expand Down
4 changes: 2 additions & 2 deletions apollo-router-core/src/query_planner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl PlanNode {
parent_value: &'a Value,
) -> future::BoxFuture<(Value, Vec<Error>)> {
Box::pin(async move {
tracing::trace!("Executing plan:\n{:#?}", self);
tracing::trace!("executing plan:\n{:#?}", self);
let mut value;
let mut errors;

Expand Down Expand Up @@ -440,7 +440,7 @@ pub(crate) mod fetch {
// because we need to take ownership of the inner value
if let Value::Object(mut map) = data {
if let Some(entities) = map.remove("_entities") {
tracing::trace!("Received entities: {:?}", &entities);
tracing::trace!("received entities: {:?}", &entities);

if let Value::Array(array) = entities {
let mut value = Value::default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ impl QueryPlanner for RouterBridgeQueryPlanner {
match planner_result {
PlannerResult::QueryPlan { node: Some(node) } => Ok(Arc::new(QueryPlan { root: node })),
PlannerResult::QueryPlan { node: None } => {
failfast_debug!("Empty query plan");
failfast_debug!("empty query plan");
Err(QueryPlannerError::EmptyPlan)
}
PlannerResult::Other => {
failfast_debug!("Unhandled planner result");
failfast_debug!("unhandled planner result");
Err(QueryPlannerError::UnhandledPlannerResult)
}
}
Expand Down
4 changes: 2 additions & 2 deletions apollo-router-core/src/services/reqwest_subgraph_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,9 @@ impl reqwest_middleware::Middleware for LoggingMiddleware {
extensions: &mut task_local_extensions::Extensions,
next: reqwest_middleware::Next<'_>,
) -> reqwest_middleware::Result<reqwest::Response> {
tracing::trace!("Request to service {}: {:?}", self.service, req);
tracing::trace!("request to service {}: {:?}", self.service, req);
let res = next.run(req, extensions).await;
tracing::trace!("Response from service {}: {:?}", self.service, res);
tracing::trace!("response from service {}: {:?}", self.service, res);
res
}
}
8 changes: 4 additions & 4 deletions apollo-router-core/src/spec/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Query {
failfast_debug!("can't find operation for {:?}", operation_name);
}
} else {
failfast_debug!("Invalid type for data in response.");
failfast_debug!("invalid type for data in response.");
}
}

Expand All @@ -73,7 +73,7 @@ impl Query {
.collect::<Vec<_>>();

if !errors.is_empty() {
failfast_debug!("Parsing error(s): {}", errors.join(", "));
failfast_debug!("parsing error(s): {}", errors.join(", "));
return None;
}

Expand Down Expand Up @@ -377,7 +377,7 @@ impl Query {
}
} else {
// the fragment should have been already checked with the schema
failfast_debug!("Missing fragment named: {}", name);
failfast_debug!("missing fragment named: {}", name);
}
}
}
Expand Down Expand Up @@ -458,7 +458,7 @@ impl Query {
self.apply_selection_set(&fragment.selection_set, input, output, schema)?;
} else {
// the fragment should have been already checked with the schema
failfast_debug!("Missing fragment named: {}", name);
failfast_debug!("missing fragment named: {}", name);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions apollo-router/src/apollo_telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ fn stats_report_key(op_name: &opentelemetry::Value, query: &str) -> String {
// If we can't parse the query, we definitely can't normalize it, so
// just return the appropriate error.
if ast.errors().len() > 0 {
tracing::warn!("Could not parse query: {}", query);
tracing::warn!("could not parse query: {}", query);
return GRAPHQL_PARSE_FAILURE.to_string();
}
let doc = ast.document();
Expand Down Expand Up @@ -487,7 +487,7 @@ fn stats_report_key(op_name: &opentelemetry::Value, query: &str) -> String {
let mut required_definitions: Vec<_> = doc.definitions().into_iter().filter(filter).collect();
tracing::debug!("required definitions: {:?}", required_definitions);
if required_definitions.len() != 1 {
tracing::warn!("Could not find required definition: {}", query);
tracing::warn!("could not find required definition: {}", query);
return GRAPHQL_UNKNOWN_OPERATION_NAME.to_string();
}
tracing::debug!(
Expand Down
26 changes: 13 additions & 13 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,31 @@ use typed_builder::TypedBuilder;
/// Configuration error.
#[derive(Debug, Error, Display)]
pub enum ConfigurationError {
/// Could not read secret from file: {0}
/// could not read secret from file: {0}
CannotReadSecretFromFile(std::io::Error),
/// Could not read secret from environment variable: {0}
/// could not read secret from environment variable: {0}
CannotReadSecretFromEnv(std::env::VarError),
/// Missing environment variable: {0}
/// missing environment variable: {0}
MissingEnvironmentVariable(String),
/// Invalid environment variable: {0}
/// invalid environment variable: {0}
InvalidEnvironmentVariable(String),
/// Could not setup OTLP tracing: {0}
/// could not setup OTLP tracing: {0}
OtlpTracing(opentelemetry::trace::TraceError),
/// The configuration could not be loaded because it requires the feature {0:?}
/// the configuration could not be loaded because it requires the feature {0:?}
MissingFeature(&'static str),
/// Unknown plugin {0}
/// unknown plugin {0}
PluginUnknown(String),
/// Plugin {plugin} could not be configured: {error}
/// plugin {plugin} could not be configured: {error}
PluginConfiguration { plugin: String, error: String },
/// Plugin {plugin} could not be started: {error}
/// plugin {plugin} could not be started: {error}
PluginStartup { plugin: String, error: String },
/// Plugin {plugin} could not be stopped: {error}
/// plugin {plugin} could not be stopped: {error}
PluginShutdown { plugin: String, error: String },
/// Unknown layer {0}
/// unknown layer {0}
LayerUnknown(String),
/// Layer {layer} could not be configured: {error}
/// layer {layer} could not be configured: {error}
LayerConfiguration { layer: String, error: String },
/// The configuration contained errors.
/// the configuration contained errors
InvalidConfiguration,
}

Expand Down
2 changes: 1 addition & 1 deletion apollo-router/src/http_server_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl HttpServerHandle {
let handle = factory
.create(router, Arc::clone(&configuration), listener)
.await?;
tracing::debug!("Restarted on {}", handle.listen_address());
tracing::debug!("restarted on {}", handle.listen_address());

Ok(handle)
}
Expand Down
46 changes: 29 additions & 17 deletions apollo-router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub use executable::{main, rt_main};
use futures::channel::{mpsc, oneshot};
use futures::prelude::*;
use futures::FutureExt;
use std::fmt::{Display, Formatter};
use std::path::{Path, PathBuf};
use std::pin::Pin;
use std::task::{Context, Poll};
Expand All @@ -42,43 +43,43 @@ type SchemaStream = Pin<Box<dyn Stream<Item = graphql::Schema> + Send>>;
/// Error types for FederatedServer.
#[derive(Error, Debug, DisplayDoc)]
pub enum FederatedServerError {
/// Failed to start server.
/// failed to start server
StartupError,

/// Failed to stop HTTP Server.
/// failed to stop HTTP Server
HttpServerLifecycleError,

/// Configuration was not supplied.
/// configuration was not supplied
NoConfiguration,

/// Schema was not supplied.
/// schema was not supplied
NoSchema,

/// Could not deserialize configuration: {0}
/// could not deserialize configuration: {0}
DeserializeConfigError(serde_yaml::Error),

/// Could not read configuration: {0}
/// could not read configuration: {0}
ReadConfigError(std::io::Error),

/// Could not read schema: {0}
/// could not read schema: {0}
ReadSchemaError(graphql::SchemaError),

/// Could not create the HTTP pipeline: {0}
/// could not create the HTTP pipeline: {0}
ServiceCreationError(tower::BoxError),

/// Could not create the HTTP server: {0}
/// could not create the HTTP server: {0}
ServerCreationError(std::io::Error),

/// Could not configure spaceport: {0}
/// could not configure spaceport: {0}
ServerSpaceportError(tokio::sync::mpsc::error::SendError<SpaceportConfig>),

/// No reload handle available
/// no reload handle available
NoReloadTracingHandleError,

/// Could not set global subscriber: {0}
/// could not set global subscriber: {0}
SetGlobalSubscriberError(SetGlobalDefaultError),

/// Could not reload tracing layer: {0}
/// could not reload tracing layer: {0}
ReloadTracingLayerError(ReloadError),
}

Expand Down Expand Up @@ -483,6 +484,17 @@ pub enum State {
Errored,
}

impl Display for State {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
State::Startup => write!(f, "startup"),
State::Running { .. } => write!(f, "running"),
State::Stopped => write!(f, "stopped"),
State::Errored => write!(f, "errored"),
}
}
}

/// A handle that allows the client to await for various server events.
pub struct FederatedServerHandle {
result: Pin<Box<dyn Future<Output = Result<(), FederatedServerError>> + Send>>,
Expand Down Expand Up @@ -549,16 +561,16 @@ impl FederatedServerHandle {
.for_each(|state| {
match state {
State::Startup => {
tracing::info!(r#"Starting Apollo Router"#)
tracing::info!("starting Apollo Router")
}
State::Running { address, .. } => {
tracing::info!("Listening on {} 🚀", address)
tracing::info!("listening on {} 🚀", address)
}
State::Stopped => {
tracing::info!("Stopped")
tracing::info!("stopped")
}
State::Errored => {
tracing::info!("Stopped with error")
tracing::info!("stopped with error")
}
}
future::ready(())
Expand Down
1 change: 0 additions & 1 deletion apollo-router/src/plugins/override_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ impl Plugin for OverrideSubgraphUrl {
type Config = HashMap<String, Url>;

fn new(configuration: Self::Config) -> Result<Self, BoxError> {
tracing::debug!("OverrideSubgraphUrl {:#?}!", configuration);
Ok(OverrideSubgraphUrl {
urls: configuration,
})
Expand Down
Loading