Skip to content

Commit

Permalink
fix(proxy): Log errors from the local proxy in auth-broker (#10659)
Browse files Browse the repository at this point in the history
Handle errors from local proxy by parsing HTTP response in auth broker
code

Closes [#19476](neondatabase/cloud#19476)
  • Loading branch information
awarus authored Feb 10, 2025
1 parent 0cf0119 commit 73633e2
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 5 deletions.
4 changes: 4 additions & 0 deletions proxy/src/auth/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ impl<T> Backend<'_, T> {
Self::Local(_) => panic!("Local backend has no API"),
}
}

pub(crate) fn is_local_proxy(&self) -> bool {
matches!(self, Self::Local(_))
}
}

impl<'a, T> Backend<'a, T> {
Expand Down
4 changes: 2 additions & 2 deletions proxy/src/serverless/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,9 @@ fn create_random_jwk() -> (SigningKey, jose_jwk::Key) {
pub(crate) enum HttpConnError {
#[error("pooled connection closed at inconsistent state")]
ConnectionClosedAbruptly(#[from] tokio::sync::watch::error::SendError<uuid::Uuid>),
#[error("could not connection to postgres in compute")]
#[error("could not connect to postgres in compute")]
PostgresConnectionError(#[from] postgres_client::Error),
#[error("could not connection to local-proxy in compute")]
#[error("could not connect to local-proxy in compute")]
LocalProxyConnectionError(#[from] LocalProxyConnError),
#[error("could not parse JWT payload")]
JwtPayloadError(serde_json::Error),
Expand Down
52 changes: 49 additions & 3 deletions proxy/src/serverless/sql_over_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ use http_body_util::{BodyExt, Full};
use hyper::body::Incoming;
use hyper::http::{HeaderName, HeaderValue};
use hyper::{header, HeaderMap, Request, Response, StatusCode};
use indexmap::IndexMap;
use postgres_client::error::{DbError, ErrorPosition, SqlState};
use postgres_client::{GenericClient, IsolationLevel, NoTls, ReadyForQueryStatus, Transaction};
use pq_proto::StartupMessageParamsBuilder;
use serde::Serialize;
use serde_json::value::RawValue;
use serde_json::Value;
use tokio::time::{self, Instant};
use tokio_util::sync::CancellationToken;
Expand Down Expand Up @@ -249,6 +251,50 @@ pub(crate) async fn handle(
let mut response = match result {
Ok(r) => {
ctx.set_success();

// Handling the error response from local proxy here
if config.authentication_config.is_auth_broker && r.status().is_server_error() {
let status = r.status();

let body_bytes = r
.collect()
.await
.map_err(|e| {
ApiError::InternalServerError(anyhow::Error::msg(format!(
"could not collect http body: {e}"
)))
})?
.to_bytes();

if let Ok(mut json_map) =
serde_json::from_slice::<IndexMap<&str, &RawValue>>(&body_bytes)
{
let message = json_map.get("message");
if let Some(message) = message {
let msg: String = match serde_json::from_str(message.get()) {
Ok(msg) => msg,
Err(_) => {
"Unable to parse the response message from server".to_string()
}
};

error!("Error response from local_proxy: {status} {msg}");

json_map.retain(|key, _| !key.starts_with("neon:")); // remove all the neon-related keys

let resp_json = serde_json::to_string(&json_map)
.unwrap_or("failed to serialize the response message".to_string());

return json_response(status, resp_json);
}
}

error!("Unable to parse the response message from local_proxy");
return json_response(
status,
json!({ "message": "Unable to parse the response message from server".to_string() }),
);
}
r
}
Err(e @ SqlOverHttpError::Cancelled(_)) => {
Expand Down Expand Up @@ -618,8 +664,6 @@ async fn handle_db_inner(

let authenticate_and_connect = Box::pin(
async {
let is_local_proxy = matches!(backend.auth_backend, crate::auth::Backend::Local(_));

let keys = match auth {
AuthData::Password(pw) => {
backend
Expand All @@ -634,7 +678,9 @@ async fn handle_db_inner(
};

let client = match keys.keys {
ComputeCredentialKeys::JwtPayload(payload) if is_local_proxy => {
ComputeCredentialKeys::JwtPayload(payload)
if backend.auth_backend.is_local_proxy() =>
{
let mut client = backend.connect_to_local_postgres(ctx, conn_info).await?;
let (cli_inner, _dsc) = client.client_inner();
cli_inner.set_jwt_session(&payload).await?;
Expand Down

1 comment on commit 73633e2

@github-actions
Copy link

Choose a reason for hiding this comment

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

7576 tests run: 7186 passed, 0 failed, 390 skipped (full report)


Flaky tests (1)

Postgres 16

Code coverage* (full report)

  • functions: 33.2% (8581 of 25835 functions)
  • lines: 49.1% (72281 of 147320 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
73633e2 at 2025-02-10T18:27:05.363Z :recycle:

Please sign in to comment.