Save incremental progress on issue / PR fetch failures #11

lschuermann · 2025-11-03T23:49:36Z

This commit implements logic for saving incremental progress when encountering an error fetching issues or PRs (such as an unsupported even type). When encountering such an error, keep fetching other issues and store the failed issue / PR IDs in the state file. Subsequent runs will then retry fetching these failed resources.

lschuermann · 2025-11-03T23:53:37Z

Thanks for building this awesome tool.

I've been using it to make backups of larger repositories, and while that works great for the most part, I'm regularly encountering failures due to unsupported event types being added by GitHub, but not handled in octocrab. Esp. when making backups manually (and fetching many issues and PRs in one run), it is extremely painful for the tool to crash mid-backup and not save incremental state (esp. given the tight rate limits on GitHub's APIs).

By saving the set of failed issues and PRs we can still make progress overall, and only re-fetch the failed ones later.

The code contains a couple TODOs still. I'm happy to address these but wanted to check in and see if this is a feature you'd like to see merged at all first.

0xB10C · 2025-11-04T01:52:38Z

Thanks for the feedback! I'll think about this more. Can you share an example of a repo with an event we don't know yet? Would like to reproduce this.

I guess, ideally octocrab wouldn't fail on events it doesn't know yet.

Being loud (i.e. failing) to process unknown events can also be seen as a feature. We know that our backup didn't work. As far as I understand your PR, we'd silently fail to backup an issue or PR if we don't support an event and don't know if we'll ever be able to fetch it in the future?

lschuermann · 2025-11-04T03:24:19Z

we'd silently fail to backup an issue or PR if we don't support an event and don't know if we'll ever be able to fetch it in the future?

No, the process would still exit with a failure code, but it'd mean that we're not wasting the API requests that have gone against the rate limit already before the failure. This is especially important for the initial (non-incremental) backup of existing, large repos. Specifically, I have wasted hours of maxing out my personal 5000 request / hour limit trying to backup a repository, running into more and more missing even type issues, before writing this patch.

lschuermann · 2025-11-04T03:25:30Z

Can you share an example of a repo with an event we don't know yet? Would like to reproduce this.

Yes, for instance https://github.com/tock/tock has a bunch of event types (such as related to merge groups) that cause this program to fail. Here's the PR to octocrab adding these event types: XAMPPRocky/octocrab#821

0xB10C · 2025-11-04T13:39:37Z

we'd silently fail to backup an issue or PR if we don't support an event and don't know if we'll ever be able to fetch it in the future?

No, the process would still exit with a failure code, but it'd mean that we're not wasting the API requests that have gone against the rate limit already before the failure. This is especially important for the initial (non-incremental) backup of existing, large repos. Specifically, I have wasted hours of maxing out my personal 5000 request / hour limit trying to backup a repository, running into more and more missing even type issues, before writing this patch.

Ah, thanks for clarifying! Then Concept ACK for saving a mid state of the state.json file. An simpler alternative to keeping the failed fetches in the state file could be to use the timestamp (maybe minus 1h or so) of the last working PR/Issue. Sort to a "we are all backed up up to date XYZ". But your current proposal is fine too.

lschuermann · 2025-11-04T15:02:31Z

An simpler alternative to keeping the failed fetches in the state file could be to use the timestamp (maybe minus 1h or so) of the last working PR/Issue. Sort to a "we are all backed up up to date XYZ". But your current proposal is fine too.

I didn't even think of that! That is indeed a simpler solution.

The diff in the current PR is not as small as I'd hoped it'd be, but conceptually this implementation is very simple: it manages to re-use most of the fetch logic (only implementing the ability to explicitly re-fetch issues not included in the timeline, which wouldn't be fetched for older, failed issues that are being retried). As we chain the paginated entries iterator with the failed issues and PR numbers means we can reuse most of the original logic for saving issues & PRs across both new and failed ones.

I'll go ahead and remove the remaining to-dos from the code then, and we can see whether we can get this into a mergeable state. 😄

EDIT: addressed the remaining TODO, and the diff looks good to me, so should be ready for review!

0xB10C

I've been testing this with

cargo run --release -- --owner tock --repo tock -d tock

and a v1 state.json file similar to

{
  "version": 1,
  "last_backup": "2025-11-01T00:00:00.000000000Z"
}

Functionality wise it seems to work as is! Would be good to fix up the logging.

0xB10C · 2025-11-10T08:43:51Z

src/main.rs

+                warn!(
+                    "Failed to fetch the following issues and PRs: {:?}",
+                    failed_issues.iter().chain(failed_pulls.iter())
+                );


This prints something like: Failed to fetch the following issues and PRs: Chain { a: Some(Iter([4410])), b: Some(Iter([4661, 4602, 4660])) }.

I think that could be cleaner by saying something like failed issues: 4410 and failed PRs 4661, 4602, 4660 directly.

Oops, yes, this is definitely an oversight. Will fix.

Fixed, it now prints a nicely formatted message indicating which issues and PRs (if any) failed.

0xB10C · 2025-11-10T08:49:57Z

src/main.rs

+                            loaded_pulls += 1;
+                        }
+                        Err(e) => {
+                            error!("Could not get pull-request #{}: {}", pr_number, e);


This seems to print the full backtrace. I'm not sure if this was always the case or if it changed with this PR...

Ideally it wouldn't do this to make the log a bit more human friendly.

ERROR github_metadata_backup] Could not get pull-request #4660: Serde Error: unknown variant `added_to_merge_queue`, expected one of `added_to_project`, `assigned`, `auto_merge_disabled`, `auto_merge_enabled`, `automatic_base_change_failed`, `automatic_base_change_succeeded`, `auto_rebase_enabled`, `auto_squash_enabled`, `base_ref_changed`, `base_ref_force_pushed`, `closed`, `commented`, `comment_deleted`, `committed`, `connected`, `convert_to_draft`, `converted_note_to_issue`, `converted_to_discussion`, `cross-referenced`, `demilestoned`, `deployed`, `deployment_environment_changed`, `disconnected`, `head_ref_deleted`, `head_ref_force_pushed`, `head_ref_restored`, `labeled`, `line-commented`, `locked`, `mentioned`, `marked_as_duplicate`, `merged`, `milestoned`, `moved_columns_in_project`, `pinned`, `ready_for_review`, `referenced`, `removed_from_project`, `renamed`, `reopened`, `review_dismissed`, `review_requested`, `review_request_removed`, `reviewed`, `subscribed`, `transferred`, `unassigned`, `unlabeled`, `unlocked`, `unmarked_as_duplicate`, `unpinned`, `unsubscribed`, `user_blocked` Found at 0: snafu::backtrace_impl::<impl snafu::GenerateImplicitData for std::backtrace::Backtrace>::generate 1: <octocrab::error::SerdeSnafu as snafu::IntoError<octocrab::error::Error>>::into_error 2: <octocrab::page::Page<T> as octocrab::from_response::FromResponse>::from_response::{{closure}} 3: github_metadata_backup::get_timeline_page::{{closure}} 4: github_metadata_backup::get_timeline::{{closure}} 5: github_metadata_backup::main::{{closure}}::{{closure}} 6: tokio::runtime::task::core::Core<T,S>::poll 7: tokio::runtime::task::raw::poll 8: tokio::runtime::scheduler::multi_thread::worker::Context::run_task 9: tokio::runtime::scheduler::multi_thread::worker::Context::run 10: tokio::runtime::context::runtime::enter_runtime 11: tokio::runtime::scheduler::multi_thread::worker::run 12: <tokio::runtime::blocking::task::BlockingTask<T> as core::future::future::Future>::poll 13: tokio::runtime::task::core::Core<T,S>::poll 14: tokio::runtime::task::raw::poll 15: tokio::runtime::blocking::pool::Inner::run 16: std::sys::backtrace::__rust_begin_short_backtrace 17: core::ops::function::FnOnce::call_once{{vtable.shim}} 18: std::sys::pal::unix::thread::Thread::new::thread_start 19: start_thread 20: __GI___clone3

I think this was always the case, and I agree it's undesirable (esp. given the backtrace doesn't actually contain that much interesting information for things like a serde error).

However, this can't be controlled from within this crate. The Display trait for octocrab errors unfortunately always includes backtraces: https://github.com/XAMPPRocky/octocrab/blob/610775a98df4a40acf1406e366962c6a73383420/src/error.rs#L80-L84

IMHO this is a questionable design choice, and I will open a PR removing these backtraces (which, I think, can still be recovered from the Error object if desired). But I'd be inclined to viewing this as somewhat separate from this PR. 😅

I created a PR removing the unconditional backtraces from octocrab's Error type: XAMPPRocky/octocrab#824 (comment)

0xB10C · 2025-11-10T08:50:36Z

src/main.rs

+                            loaded_issues += 1;
+                        }
+                        Err(e) => {
+                            error!("Could not get issue #{}: {}", issue_number, e);


Same as with PRs, prints full stack trace. Ideally it wouldn't.

Same as above, this is outside of this crate's control.

This commit implements logic for saving incremental progress when encountering an error fetching issues or PRs (such as an unsupported even type). When encountering such an error, keep fetching other issues and store the failed issue / PR IDs in the state file. The current run exit with an error code of `EXIT_API_ERROR` (3). Subsequent runs will then retry fetching these failed resources.

0xB10C · 2025-11-12T15:54:29Z

ACK

Checked that this now prints:

...
[2025-11-12T15:52:22Z INFO  github_metadata_backup] Written tock/pulls/4665.json
[2025-11-12T15:52:24Z INFO  github_metadata_backup] Written tock/pulls/4666.json
[2025-11-12T15:52:28Z INFO  github_metadata_backup] Written tock/pulls/4601.json
[2025-11-12T15:52:29Z INFO  github_metadata_backup] Written tock/issues/4634.json
[2025-11-12T15:52:31Z INFO  github_metadata_backup] Written tock/pulls/4638.json
[2025-11-12T15:52:31Z INFO  github_metadata_backup] Written tock/issues/4575.json
[2025-11-12T15:52:33Z INFO  github_metadata_backup] Loaded 4 issues and 16 pulls from tock:tock
[2025-11-12T15:52:33Z INFO  github_metadata_backup] Written tock/pulls/4529.json
[2025-11-12T15:52:33Z INFO  github_metadata_backup] Written backup state to tock/state.json
[2025-11-12T15:52:33Z WARN  github_metadata_backup] Failed to fetch issues 4410 and PRs 4661, 4602, 4660, 4583

and (still) returns error code 3, when repeating my testing from above.

0xB10C · 2025-11-12T15:54:46Z

Thank you @lschuermann!

0xB10C · 2025-11-12T15:58:33Z

Opened #12 to track updating to next octocrab release including your PRs. (Mainly so I don't forget).

lschuermann force-pushed the dev/handle-fetch-failures-gracefully branch from 2071457 to 2f8ff95 Compare November 4, 2025 15:08

0xB10C reviewed Nov 10, 2025

View reviewed changes

lschuermann force-pushed the dev/handle-fetch-failures-gracefully branch from 2f8ff95 to b54628e Compare November 10, 2025 23:01

lschuermann mentioned this pull request Nov 10, 2025

octocrab::Error: don't include unconditional backtrace in Display impl XAMPPRocky/octocrab#824

Open

0xB10C merged commit 7ba9de2 into 0xB10C:master Nov 12, 2025
1 check passed

Save incremental progress on issue / PR fetch failures #11

Save incremental progress on issue / PR fetch failures #11

Uh oh!

Conversation

lschuermann commented Nov 3, 2025

Uh oh!

lschuermann commented Nov 3, 2025

Uh oh!

0xB10C commented Nov 4, 2025

Uh oh!

lschuermann commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lschuermann commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xB10C commented Nov 4, 2025

Uh oh!

lschuermann commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xB10C left a comment

Choose a reason for hiding this comment

Uh oh!

0xB10C Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

lschuermann Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

lschuermann Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

0xB10C Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

lschuermann Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lschuermann Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

0xB10C Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

lschuermann Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

0xB10C commented Nov 12, 2025

Uh oh!

Uh oh!

0xB10C commented Nov 12, 2025

Uh oh!

0xB10C commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lschuermann commented Nov 4, 2025 •

edited

Loading

lschuermann commented Nov 4, 2025 •

edited

Loading

lschuermann commented Nov 4, 2025 •

edited

Loading

lschuermann Nov 10, 2025 •

edited

Loading