Skip to content

Commit 83ea9d9

Browse files
sypharsyphar
committed
consistency: warn & continue when errors happen while fixing inconsistencies
1 parent 091e9ab commit 83ea9d9

File tree

4 files changed

+66
-49
lines changed

4 files changed

+66
-49
lines changed

‎Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ percent-encoding = "2.2.0"
9797

9898
# NOTE: if you change this, also double-check that the comment in `queue_builder::remove_tempdirs` is still accurate.
9999
tempfile = "3.1.0"
100+
fn-error-context = "0.2.0"
100101

101102
# Templating
102103
tera = { version = "1.5.0", features = ["builtins"] }
@@ -122,7 +123,6 @@ kuchiki = "0.8"
122123
rand = "0.8"
123124
mockito = "0.31.0"
124125
test-case = "2.0.0"
125-
fn-error-context = "0.2.0"
126126
aws-smithy-client = { version = "0.53.0", features = ["test-util"]}
127127
aws-smithy-http = "0.53.0"
128128
indoc = "1.0.7"

‎src/build_queue.rs

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::storage::Storage;
66
use crate::utils::{get_config, get_crate_priority, report_error, set_config, ConfigName};
77
use crate::{Config, Index, Metrics, RustwideBuilder};
88
use anyhow::Context;
9+
use fn_error_context::context;
910

1011
use tracing::{debug, error, info, warn};
1112

@@ -69,6 +70,7 @@ impl BuildQueue {
6970
Ok(())
7071
}
7172

73+
#[context("error trying to add {name}-{version} to build queue")]
7274
pub fn add_crate(
7375
&self,
7476
name: &str,
@@ -342,15 +344,16 @@ impl BuildQueue {
342344
let yanked = change.yanked();
343345
let unyanked = change.unyanked();
344346
if let Some(release) = yanked.or(unyanked) {
345-
// FIXME: https://github.com/rust-lang/docs.rs/issues/1934
346-
// we sometimes have inconsistencies between our yank-state and
347-
// the crates.io yank state, and we don't know why yet.
348-
self.set_yanked(
347+
// FIXME: delay yanks of crates that have not yet finished building
348+
// https://github.com/rust-lang/docs.rs/issues/1934
349+
if let Err(err) = self.set_yanked(
349350
&mut conn,
350351
release.name.as_str(),
351352
release.version.as_str(),
352353
yanked.is_some(),
353-
);
354+
) {
355+
report_error(&err);
356+
}
354357

355358
if let Err(err) =
356359
cdn::queue_crate_invalidation(&mut *conn, &self.config, &release.name)
@@ -372,48 +375,48 @@ impl BuildQueue {
372375
Ok(crates_added)
373376
}
374377

375-
pub fn set_yanked(&self, conn: &mut postgres::Client, name: &str, version: &str, yanked: bool) {
378+
#[context("error trying to set {name}-{version} to yanked: {yanked}")]
379+
pub fn set_yanked(
380+
&self,
381+
conn: &mut postgres::Client,
382+
name: &str,
383+
version: &str,
384+
yanked: bool,
385+
) -> Result<()> {
376386
let activity = if yanked { "yanked" } else { "unyanked" };
377387

378-
match conn
379-
.execute(
380-
"UPDATE releases
381-
SET yanked = $3
382-
FROM crates
383-
WHERE crates.id = releases.crate_id
384-
AND name = $1
385-
AND version = $2
386-
",
387-
&[&name, &version, &yanked],
388-
)
389-
.with_context(|| format!("error while setting {}-{} to {}", name, version, activity))
390-
{
391-
Ok(rows) => {
392-
if rows != 1 {
393-
match self
394-
.has_build_queued(name, version)
395-
.context("error trying to fetch build queue")
396-
{
397-
Ok(false) => {
398-
// the rustwide builder will fetch the current yank state from
399-
// crates.io, so and missed update here will be fixed after the
400-
// build is finished.
401-
error!(
402-
"tried to yank or unyank non-existing release: {} {}",
403-
name, version
404-
);
405-
}
406-
Ok(true) => {}
407-
Err(err) => {
408-
report_error(&err);
409-
}
410-
}
411-
} else {
412-
debug!("{}-{} {}", name, version, activity);
388+
let rows = conn.execute(
389+
"UPDATE releases
390+
SET yanked = $3
391+
FROM crates
392+
WHERE crates.id = releases.crate_id
393+
AND name = $1
394+
AND version = $2",
395+
&[&name, &version, &yanked],
396+
)?;
397+
if rows != 1 {
398+
match self
399+
.has_build_queued(name, version)
400+
.context("error trying to fetch build queue")
401+
{
402+
Ok(false) => {
403+
// the rustwide builder will fetch the current yank state from
404+
// crates.io, so and missed update here will be fixed after the
405+
// build is finished.
406+
error!(
407+
"tried to yank or unyank non-existing release: {} {}",
408+
name, version
409+
);
410+
}
411+
Ok(true) => {}
412+
Err(err) => {
413+
report_error(&err);
413414
}
414415
}
415-
Err(err) => report_error(&err),
416+
} else {
417+
debug!("{}-{} {}", name, version, activity);
416418
}
419+
Ok(())
417420
}
418421

419422
/// Builds the top package from the queue. Returns whether there was a package in the queue.

‎src/db/delete.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::error::Result;
22
use crate::storage::{rustdoc_archive_path, source_archive_path, Storage};
33
use crate::{Config, Context};
44
use anyhow::Context as _;
5+
use fn_error_context::context;
56
use postgres::Client;
67
use std::fs;
78

@@ -16,6 +17,7 @@ enum CrateDeletionError {
1617
MissingCrate(String),
1718
}
1819

20+
#[context("error trying to delete crate {name} from database")]
1921
pub fn delete_crate(
2022
conn: &mut Client,
2123
storage: &Storage,
@@ -52,6 +54,7 @@ pub fn delete_crate(
5254
Ok(())
5355
}
5456

57+
#[context("error trying to delete release {name}-{version} from database")]
5558
pub fn delete_version(ctx: &dyn Context, name: &str, version: &str) -> Result<()> {
5659
let conn = &mut ctx.pool()?.get()?;
5760
let storage = ctx.storage()?;

‎src/utils/consistency/mod.rs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{build_queue, db::delete, Context};
22
use anyhow::{Context as _, Result};
33
use itertools::Itertools;
4-
use tracing::info;
4+
use tracing::{info, warn};
55

66
mod data;
77
mod db;
@@ -92,33 +92,44 @@ where
9292
match difference {
9393
diff::Difference::CrateNotInIndex(name) => {
9494
if !dry_run {
95-
delete::delete_crate(&mut conn, &storage, &config, name)?;
95+
if let Err(err) = delete::delete_crate(&mut conn, &storage, &config, name) {
96+
warn!("{:?}", err);
97+
}
9698
}
9799
result.crates_deleted += 1;
98100
}
99101
diff::Difference::CrateNotInDb(name, versions) => {
100102
for version in versions {
101103
if !dry_run {
102-
build_queue.add_crate(name, version, BUILD_PRIORITY, None)?;
104+
if let Err(err) = build_queue.add_crate(name, version, BUILD_PRIORITY, None)
105+
{
106+
warn!("{:?}", err);
107+
}
103108
}
104109
result.builds_queued += 1;
105110
}
106111
}
107112
diff::Difference::ReleaseNotInIndex(name, version) => {
108113
if !dry_run {
109-
delete::delete_version(ctx, name, version)?;
114+
if let Err(err) = delete::delete_version(ctx, name, version) {
115+
warn!("{:?}", err);
116+
}
110117
}
111118
result.releases_deleted += 1;
112119
}
113120
diff::Difference::ReleaseNotInDb(name, version) => {
114121
if !dry_run {
115-
build_queue.add_crate(name, version, BUILD_PRIORITY, None)?;
122+
if let Err(err) = build_queue.add_crate(name, version, BUILD_PRIORITY, None) {
123+
warn!("{:?}", err);
124+
}
116125
}
117126
result.builds_queued += 1;
118127
}
119128
diff::Difference::ReleaseYank(name, version, yanked) => {
120129
if !dry_run {
121-
build_queue::set_yanked(&mut conn, name, version, *yanked);
130+
if let Err(err) = build_queue::set_yanked(&mut conn, name, version, *yanked) {
131+
warn!("{:?}", err);
132+
}
122133
}
123134
result.yanks_corrected += 1;
124135
}

0 commit comments

Comments
 (0)