Skip to content

Commit

Permalink
Merge pull request #14 from Sh1nku/sh1nku/more-error-case-testing
Browse files Browse the repository at this point in the history
Improve error messages when reaching a non-solr server
  • Loading branch information
Sh1nku authored Oct 27, 2024
2 parents db9b953 + 62e93eb commit e5ab054
Show file tree
Hide file tree
Showing 29 changed files with 494 additions and 103 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test_python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
maturin develop
working-directory: ./wrappers/python
- name: Start docker containers
run: docker-compose up -d
run: docker compose up -d
working-directory: ./docker/${{ matrix.docker-version }}
- name: Run pyright
working-directory: ./wrappers/python
Expand Down
35 changes: 28 additions & 7 deletions .github/workflows/test_rust.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
name: Unit tests, linting, and formatting
on: [push]
on: [ push ]
jobs:
msrv:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Update Rust
run: |
rustup override set stable
rustup update stable
- name: Install cargo-msrv
run: cargo install cargo-msrv --locked
- name: Check MSRV
run: cargo msrv verify --all-features --path framework/
format:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Update Rust
run: |
rustup override set stable
rustup update stable
- name: Lint
run: cargo clippy
- name: Check formatting
run: cargo fmt --check

checks:
runs-on: ubuntu-latest
strategy:
Expand All @@ -13,11 +38,7 @@ jobs:
rustup override set stable
rustup update stable
- name: Start docker containers
run: docker-compose up -d
run: docker compose up -d
working-directory: ./docker/${{ matrix.docker-version }}
- name: Run tests
run: cargo test --all-features
- name: Lint
run: cargo clippy
- name: Check formatting
run: cargo fmt --check
run: cargo test --all-features
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# v0.6.0

* Breaking changes to error handling. More consistent and clearer error messages.

# v0.5.0

* Add logging of solr requests
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ members = [

[workspace.package]
edition = "2021"
version = "0.5.0"
version = "0.6.0"

[workspace.dependencies]
solrstice = { path = "framework" }
Expand Down
12 changes: 9 additions & 3 deletions docker/8_0/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ services:
ZOO_PORT: 2181
ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888'
ports:
- "2181:2181"
- "127.0.0.1:2181:2181"
restart: unless-stopped
solr1:
build:
context: .
hostname: solr1
ports:
- "8983:8983"
- "127.0.0.1:8983:8983"
volumes:
- 'solr1_varsolr:/var/solr'
environment:
Expand All @@ -27,9 +27,15 @@ services:
speedbump:
image: kffl/speedbump:latest
ports:
- "8984:8984"
- "127.0.0.1:8984:8984"
command: --latency 2s --port 8984 solr1:8983
restart: unless-stopped
error_nginx:
image: nginx:alpine
volumes:
- '../error-nginx.conf:/etc/nginx/nginx.conf'
ports:
- '127.0.0.1:8985:80'
volumes:
zoo1_data:
solr1_varsolr:
12 changes: 9 additions & 3 deletions docker/8_11/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ services:
ZOO_PORT: 2181
ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888'
ports:
- "2181:2181"
- "127.0.0.1:2181:2181"
restart: unless-stopped
solr1:
build:
context: .
hostname: solr1
ports:
- "8983:8983"
- "127.0.0.1:8983:8983"
volumes:
- 'solr1_varsolr:/var/solr'
environment:
Expand All @@ -27,9 +27,15 @@ services:
speedbump:
image: kffl/speedbump:latest
ports:
- "8984:8984"
- "127.0.0.1:8984:8984"
command: --latency 2s --port 8984 solr1:8983
restart: unless-stopped
error_nginx:
image: nginx:alpine
volumes:
- '../error-nginx.conf:/etc/nginx/nginx.conf'
ports:
- '127.0.0.1:8985:80'
volumes:
zoo1_data:
solr1_varsolr:
12 changes: 9 additions & 3 deletions docker/9_0/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ services:
ZOO_PORT: 2181
ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888'
ports:
- "2181:2181"
- "127.0.0.1:2181:2181"
restart: unless-stopped
solr1:
build:
context: .
hostname: solr1
ports:
- "8983:8983"
- "127.0.0.1:8983:8983"
volumes:
- 'solr1_varsolr:/var/solr'
environment:
Expand All @@ -27,9 +27,15 @@ services:
speedbump:
image: kffl/speedbump:latest
ports:
- "8984:8984"
- "127.0.0.1:8984:8984"
command: --latency 2s --port 8984 solr1:8983
restart: unless-stopped
error_nginx:
image: nginx:alpine
volumes:
- '../error-nginx.conf:/etc/nginx/nginx.conf'
ports:
- '127.0.0.1:8985:80'
volumes:
zoo1_data:
solr1_varsolr:
12 changes: 9 additions & 3 deletions docker/9_3/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ services:
ZOO_PORT: 2181
ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888'
ports:
- "2181:2181"
- "127.0.0.1:2181:2181"
restart: unless-stopped
solr1:
build:
context: .
hostname: solr1
ports:
- "8983:8983"
- "127.0.0.1:8983:8983"
volumes:
- 'solr1_varsolr:/var/solr'
environment:
Expand All @@ -27,9 +27,15 @@ services:
speedbump:
image: kffl/speedbump:latest
ports:
- "8984:8984"
- "127.0.0.1:8984:8984"
command: --latency 2s --port 8984 solr1:8983
restart: unless-stopped
error_nginx:
image: nginx:alpine
volumes:
- '../error-nginx.conf:/etc/nginx/nginx.conf'
ports:
- '127.0.0.1:8985:80'
volumes:
zoo1_data:
solr1_varsolr:
22 changes: 22 additions & 0 deletions docker/error-nginx.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
events {}

http {
server {
listen 80;
location /solr/notfound_collection/ {
return 404 "Collection not found";
}

location /solr/error_collection/ {
return 500 "Internal Server Error";
}

location /solr/always_200/ {
return 200 "Always 200";
}

location /status {
return 200 "We are up";
}
}
}
2 changes: 1 addition & 1 deletion docs/src/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ The library is written in Rust, and has a wrapper to Python. Both async and bloc
### Rust
You can install the library by putting this in your `Cargo.toml`
```toml
solrstice = { version = "0.5.0", features = ["blocking"] }
solrstice = { version = "0.6.0", features = ["blocking"] }
```
If the `blocking` feature is not provided, only async will work.
* [Rust mdBook docs]()
Expand Down
1 change: 1 addition & 0 deletions framework/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ keywords = ["solr", "search"]
categories = ["api-bindings"]
readme = "README.md"
repository = "https://github.com/Sh1nku/solrstice"
rust-version = "1.73.0"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand Down
17 changes: 10 additions & 7 deletions framework/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,20 @@ pub enum Error {

#[error(transparent)]
SerdeJsonError(#[from] serde_json::Error),
#[error("Error from Solr {code:?}: {msg:?}")]
SolrResponseError { code: usize, msg: String },
#[error("Authentication error: {0}")]
SolrAuthError(String),
#[error(transparent)]
ZkError(#[from] zookeeper_async::ZkError),

#[error(transparent)]
StripPrefixError(#[from] std::path::StripPrefixError),

#[error("Solr Connection error: {0}")]
SolrConnectionError(String),
#[error("Solr setup error: {0}")]
SolrSetupError(String),
#[error("Solr connection error: {code:?} - {url:?}\n{msg:?}")]
SolrConnectionError { code: u16, url: String, msg: String },
#[error("Solr response error: {code:?} - {url:?}\n{msg:?}")]
SolrResponseError { code: u16, url: String, msg: String },
#[error("Solr auth error: {code:?} - {url:?}\n{msg:?}")]
SolrAuthError { code: u16, url: String, msg: String },

#[error("Unknown error: {0}")]
Unknown(String),
Expand All @@ -37,7 +39,7 @@ impl From<&str> for Error {
}

/// Helper function to check if a SolrResponse contains an error
pub fn try_solr_error(response: &SolrResponse) -> Result<(), Error> {
pub fn try_solr_error(url: String, response: &SolrResponse) -> Result<(), Error> {
match &response.error {
None => Ok(()),
Some(err) => {
Expand All @@ -49,6 +51,7 @@ pub fn try_solr_error(response: &SolrResponse) -> Result<(), Error> {
}
Err(Error::SolrResponseError {
code: err.code,
url,
msg,
})
}
Expand Down
8 changes: 2 additions & 6 deletions framework/src/hosts/solr_server_host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ impl SolrHost for SolrMultipleServerHost {
async fn get_solr_node(&self) -> Result<Cow<str>, Error> {
let mut server_indices: Vec<usize> = (0..self.hosts.len()).collect();
if server_indices.is_empty() {
return Err(Error::SolrConnectionError(
"No Solr Host Specified".to_string(),
));
return Err(Error::SolrSetupError("No Solr Host Specified".to_string()));
}
fastrand::shuffle(&mut server_indices);
for i in server_indices {
Expand All @@ -82,9 +80,7 @@ impl SolrHost for SolrMultipleServerHost {
}
}
}
Err(Error::SolrConnectionError(
"No Solr Host answered".to_string(),
))
Err(Error::SolrSetupError("No Solr Host answered".to_string()))
}
}

Expand Down
2 changes: 1 addition & 1 deletion framework/src/hosts/zookeeper_host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl SolrHost for ZookeeperEnsembleHost {
async fn get_solr_node(&self) -> Result<Cow<str>, Error> {
let hosts = get_hosts_from_zookeeper(&self.client).await?;
match hosts.get(fastrand::usize(0..hosts.len())) {
None => Err(Error::SolrConnectionError(
None => Err(Error::SolrSetupError(
"No ready Solr nodes from Zookeeper".to_string(),
)),
//TODO Investigate this further. Is it always http://, and do people use auth?
Expand Down
2 changes: 1 addition & 1 deletion framework/src/models/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub struct SolrResponseError {
/// The trace of the error.
pub trace: Option<String>,
/// The code of the error.
pub code: usize,
pub code: u16,
}

fn default_true() -> bool {
Expand Down
Loading

0 comments on commit e5ab054

Please sign in to comment.