Skip to content

Commit 4b4d446

Browse files
authored
Return missing instead of internal error for checkpoints that haven't been fully stored yet (#24244)
## Description In the indexer logs we see `Retrying due to error: Failed to fetch checkpoint 211394366: status: 'Internal error'` at the tip-of-chain when a checkpoint hasn't been fully stored yet. This should be a "Not Found" rather than an "Internal Error." This is especially important because the indexing framework retries with exponential back off on internal errors but linear back off on "Not Found" so returning an Internal Error is actually increasing lag. ## Test plan None really 🙈. It's kind of tough to write a unit test because this requires calling the API at the exact instant where the checkpoint exists but the effects haven't been stored yet. ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [x] gRPC: Return "Not Found" for new checkpoints that haven't been fully stored yet instead of "Internal Error."
1 parent 9454c23 commit 4b4d446

File tree

2 files changed

+20
-16
lines changed

2 files changed

+20
-16
lines changed

crates/sui-rpc-api/src/error.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,15 @@ impl From<RpcError> for tonic::Status {
6565

6666
impl From<sui_types::storage::error::Error> for RpcError {
6767
fn from(value: sui_types::storage::error::Error) -> Self {
68+
use sui_types::storage::error::Kind;
69+
70+
let code = match value.kind() {
71+
Kind::Missing => Code::NotFound,
72+
_ => Code::Internal,
73+
};
74+
6875
Self {
69-
code: Code::Internal,
76+
code,
7077
message: Some(value.to_string()),
7178
details: None,
7279
}

crates/sui-types/src/storage/read_store.rs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,13 +164,13 @@ pub trait ReadStore: ObjectStore {
164164
) -> Option<FullCheckpointContents>;
165165

166166
// Fetch all checkpoint data
167-
// TODO fix return type to not be anyhow
168167
fn get_checkpoint_data(
169168
&self,
170169
checkpoint: VerifiedCheckpoint,
171170
checkpoint_contents: CheckpointContents,
172-
) -> anyhow::Result<Checkpoint> {
171+
) -> Result<Checkpoint> {
173172
use crate::effects::TransactionEffectsAPI;
173+
use crate::storage::error::Error;
174174
use std::collections::HashMap;
175175

176176
let transaction_digests = checkpoint_contents
@@ -181,15 +181,15 @@ pub trait ReadStore: ObjectStore {
181181
.multi_get_transactions(&transaction_digests)
182182
.into_iter()
183183
.map(|maybe_transaction| {
184-
maybe_transaction.ok_or_else(|| anyhow::anyhow!("missing transaction"))
184+
maybe_transaction.ok_or_else(|| Error::missing("missing transaction"))
185185
})
186-
.collect::<anyhow::Result<Vec<_>>>()?;
186+
.collect::<Result<Vec<_>>>()?;
187187

188188
let effects = self
189189
.multi_get_transaction_effects(&transaction_digests)
190190
.into_iter()
191-
.map(|maybe_effects| maybe_effects.ok_or_else(|| anyhow::anyhow!("missing effects")))
192-
.collect::<anyhow::Result<Vec<_>>>()?;
191+
.map(|maybe_effects| maybe_effects.ok_or_else(|| Error::missing("missing effects")))
192+
.collect::<Result<Vec<_>>>()?;
193193

194194
let event_tx_digests = effects
195195
.iter()
@@ -202,10 +202,10 @@ pub trait ReadStore: ObjectStore {
202202
.zip(event_tx_digests)
203203
.map(|(maybe_event, tx_digest)| {
204204
maybe_event
205-
.ok_or_else(|| anyhow::anyhow!("missing event for tx {tx_digest}"))
205+
.ok_or_else(|| Error::missing(format!("missing event for tx {tx_digest}")))
206206
.map(|event| (tx_digest, event))
207207
})
208-
.collect::<anyhow::Result<HashMap<_, _>>>()?;
208+
.collect::<Result<HashMap<_, _>>>()?;
209209

210210
let mut transactions = Vec::with_capacity(txns.len());
211211
for (tx, fx) in txns.into_iter().zip(effects) {
@@ -247,10 +247,7 @@ pub trait ReadStore: ObjectStore {
247247
let mut object_set = ObjectSet::default();
248248
for (idx, object) in objects.into_iter().enumerate() {
249249
object_set.insert(object.ok_or_else(|| {
250-
crate::storage::error::Error::custom(format!(
251-
"unabled to load object {:?}",
252-
refs[idx]
253-
))
250+
Error::missing(format!("unable to load object {:?}", refs[idx]))
254251
})?);
255252
}
256253
object_set
@@ -380,7 +377,7 @@ impl<T: ReadStore + ?Sized> ReadStore for &T {
380377
&self,
381378
checkpoint: VerifiedCheckpoint,
382379
checkpoint_contents: CheckpointContents,
383-
) -> anyhow::Result<Checkpoint> {
380+
) -> Result<Checkpoint> {
384381
(*self).get_checkpoint_data(checkpoint, checkpoint_contents)
385382
}
386383
}
@@ -498,7 +495,7 @@ impl<T: ReadStore + ?Sized> ReadStore for Box<T> {
498495
&self,
499496
checkpoint: VerifiedCheckpoint,
500497
checkpoint_contents: CheckpointContents,
501-
) -> anyhow::Result<Checkpoint> {
498+
) -> Result<Checkpoint> {
502499
(**self).get_checkpoint_data(checkpoint, checkpoint_contents)
503500
}
504501
}
@@ -616,7 +613,7 @@ impl<T: ReadStore + ?Sized> ReadStore for Arc<T> {
616613
&self,
617614
checkpoint: VerifiedCheckpoint,
618615
checkpoint_contents: CheckpointContents,
619-
) -> anyhow::Result<Checkpoint> {
616+
) -> Result<Checkpoint> {
620617
(**self).get_checkpoint_data(checkpoint, checkpoint_contents)
621618
}
622619
}

0 commit comments

Comments
 (0)