Skip to content

Commit

Permalink
Merge pull request #300 from nyx-space/245-improve-error-handling
Browse files Browse the repository at this point in the history
Improve error handling
  • Loading branch information
ChristopherRabotin authored May 10, 2024
2 parents f9d4100 + bd1e70d commit c88a3a0
Show file tree
Hide file tree
Showing 24 changed files with 462 additions and 246 deletions.
18 changes: 11 additions & 7 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,32 @@ jobs:
pytest
macos:
runs-on: macos-latest
runs-on: ${{ matrix.platform.runner }}
strategy:
matrix:
target: [x86_64, aarch64]
platform:
- runner: macos-latest
target: x86_64
- runner: macos-14
target: aarch64
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.11'
python-version: '3.10'
- name: Build wheels
uses: PyO3/maturin-action@v1
with:
target: ${{ matrix.target }}
target: ${{ matrix.platform.target }}
args: --release --out dist --find-interpreter -F python
sccache: 'true'
- name: Upload wheels
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
with:
name: wheels
name: wheels-macos-${{ matrix.platform.target }}
path: dist
- name: pytest
if: ${{ !startsWith(matrix.target, 'aarch64') }}
if: ${{ !startsWith(matrix.platform.target, 'aarch64') }}
shell: bash
run: |
set -e
Expand Down
23 changes: 12 additions & 11 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ reqwest = { version = "0.12", features = ["blocking", "json"], optional = true }
tabled = { version = "0.15.0", optional = true }
openssl = { version = "0.10", features = ["vendored"], optional = true }
web-time = { version = "1.0.0", optional = true }
snafu = { version = "0.8.2", default-features = false }

[features]
default = ["std"]
std = ["serde", "serde_derive", "web-time", "snafu/std", "snafu/backtrace"]
python = ["std", "pyo3", "ut1"]
ut1 = ["std", "reqwest", "tabled", "openssl"]

[dev-dependencies]
serde_json = "1.0.91"
criterion = "0.5.1"
iai = "0.1"

[target.wasm32-unknown-unknown.dependencies]
js-sys = { version = "0.3" }
Expand Down Expand Up @@ -63,17 +75,6 @@ web-sys = { version = "0.3", features = [
'PerformanceTiming',
] }

[dev-dependencies]
serde_json = "1.0.91"
criterion = "0.5.1"
iai = "0.1"

[features]
default = ["std"]
std = ["serde", "serde_derive", "web-time"]
python = ["std", "pyo3", "ut1"]
ut1 = ["std", "reqwest", "tabled", "openssl"]

[[bench]]
name = "crit_epoch"
harness = false
Expand Down
4 changes: 2 additions & 2 deletions src/duration/kani_verif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

// Here lives all of the formal verification for Duration.

use super::{Duration, Errors};
use super::{Duration, DurationError};
use crate::NANOSECONDS_PER_CENTURY;

use kani::Arbitrary;
Expand Down Expand Up @@ -43,7 +43,7 @@ fn formal_duration_truncated_ns_reciprocity() {
// Then it does not fit on a i64, so this function should return an error
assert_eq!(
dur_from_part.try_truncated_nanoseconds(),
Err(Errors::Overflow)
Err(DurationError::Overflow)
);
} else if centuries == -1 {
// If we are negative by just enough that the centuries is negative, then the truncated seconds
Expand Down
19 changes: 14 additions & 5 deletions src/duration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
* Documentation: https://nyxspace.com/
*/

use crate::{Errors, SECONDS_PER_CENTURY, SECONDS_PER_DAY, SECONDS_PER_HOUR, SECONDS_PER_MINUTE};
use crate::errors::DurationError;
use crate::{
EpochError, SECONDS_PER_CENTURY, SECONDS_PER_DAY, SECONDS_PER_HOUR, SECONDS_PER_MINUTE,
};

pub use crate::{Freq, Frequencies, TimeUnits, Unit};

Expand Down Expand Up @@ -371,21 +374,27 @@ impl Duration {
}

/// Returns the truncated nanoseconds in a signed 64 bit integer, if the duration fits.
pub fn try_truncated_nanoseconds(&self) -> Result<i64, Errors> {
pub fn try_truncated_nanoseconds(&self) -> Result<i64, EpochError> {
// If it fits, we know that the nanoseconds also fit. abs() will fail if the centuries are min'ed out.
if self.centuries == i16::MIN || self.centuries.abs() >= 3 {
Err(Errors::Overflow)
Err(EpochError::Duration {
source: DurationError::Underflow,
})
} else if self.centuries == -1 {
Ok(-((NANOSECONDS_PER_CENTURY - self.nanoseconds) as i64))
} else if self.centuries >= 0 {
match i64::from(self.centuries).checked_mul(NANOSECONDS_PER_CENTURY as i64) {
Some(centuries_as_ns) => {
match centuries_as_ns.checked_add(self.nanoseconds as i64) {
Some(truncated_ns) => Ok(truncated_ns),
None => Err(Errors::Overflow),
None => Err(EpochError::Duration {
source: DurationError::Overflow,
}),
}
}
None => Err(Errors::Overflow),
None => Err(EpochError::Duration {
source: DurationError::Underflow,
}),
}
} else {
// Centuries negative by a decent amount
Expand Down
54 changes: 41 additions & 13 deletions src/duration/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
*/

use super::{Duration, Unit};
use crate::{Errors, ParsingErrors};
use crate::{EpochError, ParsingError};
use core::str::FromStr;

impl FromStr for Duration {
type Err = Errors;
type Err = EpochError;

/// Attempts to convert a simple string to a Duration. Does not yet support complicated durations.
///
Expand Down Expand Up @@ -53,7 +53,10 @@ impl FromStr for Duration {
let s = s_in.trim();

if s.is_empty() {
return Err(Errors::ParseError(ParsingErrors::ValueError));
return Err(EpochError::Parse {
source: ParsingError::NothingToParse,
details: "input string is empty",
});
}

// There is at least one character, so we can unwrap this.
Expand All @@ -75,13 +78,21 @@ impl FromStr for Duration {
1
} else {
// This invalid
return Err(Errors::ParseError(ParsingErrors::ValueError));
return Err(EpochError::Parse {
source: ParsingError::InvalidTimezone,
details: "invalid timezone format [+/-]HH:MM",
});
};

// Fetch the hours
let hours: i64 = match lexical_core::parse(s[indexes.0..indexes.1].as_bytes()) {
Ok(val) => val,
Err(_) => return Err(Errors::ParseError(ParsingErrors::ValueError)),
Err(err) => {
return Err(EpochError::Parse {
source: ParsingError::Lexical { err },
details: "invalid hours",
})
}
};

let mut minutes: i64 = 0;
Expand All @@ -95,22 +106,28 @@ impl FromStr for Duration {
// Fetch the minutes
match lexical_core::parse(subs.as_bytes()) {
Ok(val) => minutes = val,
Err(_) => return Err(Errors::ParseError(ParsingErrors::ValueError)),
Err(_) => {
return Err(EpochError::Parse {
source: ParsingError::ValueError,
details: "invalid minute",
})
}
}

match s.get(indexes.2 + 2 * colon..) {
None => {
// Do nothing, there are no seconds inthis offset
// Do nothing, there are no seconds in this offset
}
Some(subs) => {
if !subs.is_empty() {
// Fetch the seconds
match lexical_core::parse(subs.as_bytes()) {
Ok(val) => seconds = val,
Err(_) => {
return Err(Errors::ParseError(
ParsingErrors::ValueError,
))
return Err(EpochError::Parse {
source: ParsingError::ValueError,
details: "invalid seconds",
})
}
}
}
Expand All @@ -137,12 +154,20 @@ impl FromStr for Duration {
if seeking_number {
if prev_idx == idx {
// We've reached the end of the string and it didn't end with a unit
return Err(Errors::ParseError(ParsingErrors::UnknownOrMissingUnit));
return Err(EpochError::Parse {
source: ParsingError::UnknownOrMissingUnit,
details: "expect a unit after a numeric",
});
}
// We've found a new space so let's parse whatever precedes it
match lexical_core::parse(s[prev_idx..idx].as_bytes()) {
Ok(val) => latest_value = val,
Err(_) => return Err(Errors::ParseError(ParsingErrors::ValueError)),
Err(_) => {
return Err(EpochError::Parse {
source: ParsingError::ValueError,
details: "could not parse what precedes the space",
})
}
}
// We'll now seek a unit
seeking_number = false;
Expand All @@ -158,7 +183,10 @@ impl FromStr for Duration {
"us" | "microsecond" | "microseconds" => 5,
"ns" | "nanosecond" | "nanoseconds" => 6,
_ => {
return Err(Errors::ParseError(ParsingErrors::UnknownOrMissingUnit));
return Err(EpochError::Parse {
source: ParsingError::UnknownOrMissingUnit,
details: "unknown unit",
});
}
};
// Store the value
Expand Down
Loading

0 comments on commit c88a3a0

Please sign in to comment.