Skip to content

Report the right count when req_perform_sequential() is interrupted - #893

Merged
hadley merged 3 commits into
r-lib:mainfrom
m-muecke:fix/sequential-interrupt-count
Oct 8, 2026
Merged

hadley merged 3 commits into
r-lib:mainfrom
m-muecke:fix/sequential-interrupt-count

Conversation

@m-muecke

@m-muecke m-muecke commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

matchs req_perform_iterative()

@m-muecke
m-muecke force-pushed the fix/sequential-interrupt-count branch from ee34aa7 to aef991b Compare October 7, 2026 08:26
check_repeated_interrupt()

resps <- resps[seq_len(i)]
# interrupt might occur before request i completes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to truncate resps still?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this never had any effect, since it ran in the tryCatch handler and the local copy was never used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah resps <<- resps[seq_len(i)] was likely the intent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes more sense, that would follow req_perform_iterative() and truncate as well

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, that sounds like the goal.

@hadley

hadley commented Oct 7, 2026

Copy link
Copy Markdown
Member

Would you mind having a stab at writing a test for this? It's totally possible that it won't be worth it and then I'll ask you to delete it 😄, but I'd like to see what claude/chatgpt suggests.

@m-muecke

m-muecke commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

no problem, done

@hadley
hadley merged commit 804cfc9 into r-lib:main Oct 8, 2026
13 checks passed
@hadley

hadley commented Oct 8, 2026

Copy link
Copy Markdown
Member

That wasn't bad at all 😆

@m-muecke
m-muecke deleted the fix/sequential-interrupt-count branch October 8, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants