use typed errors in /view.mp4 path

This fixes #178. Before, everything got translated to 5xx status;
now it produces the correct type in several cases.

Ideally I'd get rid of the untyped errors in all of web.rs; this is
a small step.
This commit is contained in:
Scott Lamb 2021-10-21 10:25:37 -07:00
parent 97bfe0afc3
commit 7b0099fb4e
4 changed files with 68 additions and 48 deletions

View File

@ -462,7 +462,7 @@ Bugs and limitations:
reference them. reference them.
* The final recording in every "run" ends with a frame that has duration 0. * The final recording in every "run" ends with a frame that has duration 0.
It's not possible to append additional segments after such a frame; It's not possible to append additional segments after such a frame;
the server will return an error like `Invalid argument: unable to append the server will return a 400 error like `Invalid argument: unable to append
recording 2/16672 after recording 2/16671 with trailing zero`. See also recording 2/16672 after recording 2/16671 with trailing zero`. See also
`hasTrailingZero` above, and `hasTrailingZero` above, and
[#178](https://github.com/scottlamb/moonfire-nvr/issues/178). [#178](https://github.com/scottlamb/moonfire-nvr/issues/178).

View File

@ -33,6 +33,7 @@ use crate::raw;
use crate::recording; use crate::recording;
use crate::schema; use crate::schema;
use crate::signal; use crate::signal;
use base::bail_t;
use base::clock::{self, Clocks}; use base::clock::{self, Clocks};
use base::strutil::encode_size; use base::strutil::encode_size;
use failure::{bail, format_err, Error, ResultExt}; use failure::{bail, format_err, Error, ResultExt};
@ -1258,10 +1259,10 @@ impl LockedDatabase {
&self, &self,
stream_id: i32, stream_id: i32,
desired_time: Range<recording::Time>, desired_time: Range<recording::Time>,
f: &mut dyn FnMut(ListRecordingsRow) -> Result<(), Error>, f: &mut dyn FnMut(ListRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), Error> { ) -> Result<(), base::Error> {
let s = match self.streams_by_id.get(&stream_id) { let s = match self.streams_by_id.get(&stream_id) {
None => bail!("no such stream {}", stream_id), None => bail_t!(NotFound, "no such stream {}", stream_id),
Some(s) => s, Some(s) => s,
}; };
raw::list_recordings_by_time(&self.conn, stream_id, desired_time.clone(), f)?; raw::list_recordings_by_time(&self.conn, stream_id, desired_time.clone(), f)?;
@ -1291,10 +1292,10 @@ impl LockedDatabase {
&self, &self,
stream_id: i32, stream_id: i32,
desired_ids: Range<i32>, desired_ids: Range<i32>,
f: &mut dyn FnMut(ListRecordingsRow) -> Result<(), Error>, f: &mut dyn FnMut(ListRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), Error> { ) -> Result<(), base::Error> {
let s = match self.streams_by_id.get(&stream_id) { let s = match self.streams_by_id.get(&stream_id) {
None => bail!("no such stream {}", stream_id), None => bail_t!(NotFound, "no such stream {}", stream_id),
Some(s) => s, Some(s) => s,
}; };
if desired_ids.start < s.cum_recordings { if desired_ids.start < s.cum_recordings {
@ -1332,8 +1333,8 @@ impl LockedDatabase {
stream_id: i32, stream_id: i32,
desired_time: Range<recording::Time>, desired_time: Range<recording::Time>,
forced_split: recording::Duration, forced_split: recording::Duration,
f: &mut dyn FnMut(&ListAggregatedRecordingsRow) -> Result<(), Error>, f: &mut dyn FnMut(&ListAggregatedRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), Error> { ) -> Result<(), base::Error> {
// Iterate, maintaining a map from a recording_id to the aggregated row for the latest // Iterate, maintaining a map from a recording_id to the aggregated row for the latest
// batch of recordings from the run starting at that id. Runs can be split into multiple // batch of recordings from the run starting at that id. Runs can be split into multiple
// batches for a few reasons: // batches for a few reasons:
@ -1371,7 +1372,8 @@ impl LockedDatabase {
} else { } else {
// append. // append.
if a.time.end != row.start { if a.time.end != row.start {
bail!( bail_t!(
Internal,
"stream {} recording {} ends at {} but {} starts at {}", "stream {} recording {} ends at {} but {} starts at {}",
stream_id, stream_id,
a.ids.end - 1, a.ids.end - 1,
@ -1381,7 +1383,8 @@ impl LockedDatabase {
); );
} }
if a.open_id != row.open_id { if a.open_id != row.open_id {
bail!( bail_t!(
Internal,
"stream {} recording {} has open id {} but {} has {}", "stream {} recording {} has open id {} but {} has {}",
stream_id, stream_id,
a.ids.end - 1, a.ids.end - 1,

View File

@ -6,7 +6,8 @@
use crate::db::{self, CompositeId, FromSqlUuid}; use crate::db::{self, CompositeId, FromSqlUuid};
use crate::recording; use crate::recording;
use failure::{bail, Error, ResultExt}; use base::{ErrorKind, ResultExt as _};
use failure::{bail, Error, ResultExt as _};
use fnv::FnvHashSet; use fnv::FnvHashSet;
use rusqlite::{named_params, params}; use rusqlite::{named_params, params};
use std::ops::Range; use std::ops::Range;
@ -103,14 +104,18 @@ pub(crate) fn list_recordings_by_time(
conn: &rusqlite::Connection, conn: &rusqlite::Connection,
stream_id: i32, stream_id: i32,
desired_time: Range<recording::Time>, desired_time: Range<recording::Time>,
f: &mut dyn FnMut(db::ListRecordingsRow) -> Result<(), Error>, f: &mut dyn FnMut(db::ListRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), Error> { ) -> Result<(), base::Error> {
let mut stmt = conn.prepare_cached(LIST_RECORDINGS_BY_TIME_SQL)?; let mut stmt = conn
let rows = stmt.query(named_params! { .prepare_cached(LIST_RECORDINGS_BY_TIME_SQL)
.err_kind(ErrorKind::Internal)?;
let rows = stmt
.query(named_params! {
":stream_id": stream_id, ":stream_id": stream_id,
":start_time_90k": desired_time.start.0, ":start_time_90k": desired_time.start.0,
":end_time_90k": desired_time.end.0, ":end_time_90k": desired_time.end.0,
})?; })
.err_kind(ErrorKind::Internal)?;
list_recordings_inner(rows, false, f) list_recordings_inner(rows, false, f)
} }
@ -119,39 +124,46 @@ pub(crate) fn list_recordings_by_id(
conn: &rusqlite::Connection, conn: &rusqlite::Connection,
stream_id: i32, stream_id: i32,
desired_ids: Range<i32>, desired_ids: Range<i32>,
f: &mut dyn FnMut(db::ListRecordingsRow) -> Result<(), Error>, f: &mut dyn FnMut(db::ListRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), Error> { ) -> Result<(), base::Error> {
let mut stmt = conn.prepare_cached(LIST_RECORDINGS_BY_ID_SQL)?; let mut stmt = conn
let rows = stmt.query(named_params! { .prepare_cached(LIST_RECORDINGS_BY_ID_SQL)
.err_kind(ErrorKind::Internal)?;
let rows = stmt
.query(named_params! {
":start": CompositeId::new(stream_id, desired_ids.start).0, ":start": CompositeId::new(stream_id, desired_ids.start).0,
":end": CompositeId::new(stream_id, desired_ids.end).0, ":end": CompositeId::new(stream_id, desired_ids.end).0,
})?; })
.err_kind(ErrorKind::Internal)?;
list_recordings_inner(rows, true, f) list_recordings_inner(rows, true, f)
} }
fn list_recordings_inner( fn list_recordings_inner(
mut rows: rusqlite::Rows, mut rows: rusqlite::Rows,
include_prev: bool, include_prev: bool,
f: &mut dyn FnMut(db::ListRecordingsRow) -> Result<(), Error>, f: &mut dyn FnMut(db::ListRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), Error> { ) -> Result<(), base::Error> {
while let Some(row) = rows.next()? { while let Some(row) = rows.next().err_kind(ErrorKind::Internal)? {
let wall_duration_90k = row.get(4)?; let wall_duration_90k = row.get(4).err_kind(ErrorKind::Internal)?;
let media_duration_delta_90k: i32 = row.get(5)?; let media_duration_delta_90k: i32 = row.get(5).err_kind(ErrorKind::Internal)?;
f(db::ListRecordingsRow { f(db::ListRecordingsRow {
id: CompositeId(row.get(0)?), id: CompositeId(row.get(0).err_kind(ErrorKind::Internal)?),
run_offset: row.get(1)?, run_offset: row.get(1).err_kind(ErrorKind::Internal)?,
flags: row.get(2)?, flags: row.get(2).err_kind(ErrorKind::Internal)?,
start: recording::Time(row.get(3)?), start: recording::Time(row.get(3).err_kind(ErrorKind::Internal)?),
wall_duration_90k, wall_duration_90k,
media_duration_90k: wall_duration_90k + media_duration_delta_90k, media_duration_90k: wall_duration_90k + media_duration_delta_90k,
sample_file_bytes: row.get(6)?, sample_file_bytes: row.get(6).err_kind(ErrorKind::Internal)?,
video_samples: row.get(7)?, video_samples: row.get(7).err_kind(ErrorKind::Internal)?,
video_sync_samples: row.get(8)?, video_sync_samples: row.get(8).err_kind(ErrorKind::Internal)?,
video_sample_entry_id: row.get(9)?, video_sample_entry_id: row.get(9).err_kind(ErrorKind::Internal)?,
open_id: row.get(10)?, open_id: row.get(10).err_kind(ErrorKind::Internal)?,
prev_media_duration_and_runs: match include_prev { prev_media_duration_and_runs: match include_prev {
false => None, false => None,
true => Some((recording::Duration(row.get(11)?), row.get(12)?)), true => Some((
recording::Duration(row.get(11).err_kind(ErrorKind::Internal)?),
row.get(12).err_kind(ErrorKind::Internal)?,
)),
}, },
})?; })?;
} }

View File

@ -11,7 +11,7 @@ use core::borrow::Borrow;
use core::str::FromStr; use core::str::FromStr;
use db::dir::SampleFileDir; use db::dir::SampleFileDir;
use db::{auth, recording}; use db::{auth, recording};
use failure::{bail, format_err, Error}; use failure::{format_err, Error};
use fnv::FnvHashMap; use fnv::FnvHashMap;
use futures::stream::StreamExt; use futures::stream::StreamExt;
use futures::{future::Either, sink::SinkExt}; use futures::{future::Either, sink::SinkExt};
@ -860,7 +860,8 @@ impl Service {
if let Some(o) = s.open_id { if let Some(o) = s.open_id {
if r.open_id != o { if r.open_id != o {
bail!( bail_t!(
NotFound,
"recording {} has open id {}, requested {}", "recording {} has open id {}, requested {}",
r.id, r.id,
r.open_id, r.open_id,
@ -872,9 +873,14 @@ impl Service {
// Check for missing recordings. // Check for missing recordings.
match prev { match prev {
None if recording_id == s.ids.start => {} None if recording_id == s.ids.start => {}
None => bail!("no such recording {}/{}", stream_id, s.ids.start), None => bail_t!(
NotFound,
"no such recording {}/{}",
stream_id,
s.ids.start
),
Some(id) if r.id.recording() != id + 1 => { Some(id) if r.id.recording() != id + 1 => {
bail!("no such recording {}/{}", stream_id, id + 1); bail_t!(NotFound, "no such recording {}/{}", stream_id, id + 1);
} }
_ => {} _ => {}
}; };
@ -913,8 +919,7 @@ impl Service {
} }
cur_off += wd; cur_off += wd;
Ok(()) Ok(())
}) })?;
.map_err(internal_server_err)?;
// Check for missing recordings. // Check for missing recordings.
match prev { match prev {