Skip to content

fix response clone bug#1565

Closed
KaiPrince wants to merge 58 commits into
ProjectEvergreen:feature/issue-1247-standalone-markdown-pluginfrom
KaiPrince:kprince/fix-response-clone-bug
Closed

fix response clone bug#1565
KaiPrince wants to merge 58 commits into
ProjectEvergreen:feature/issue-1247-standalone-markdown-pluginfrom
KaiPrince:kprince/fix-response-clone-bug

Conversation

@KaiPrince

Copy link
Copy Markdown
Contributor

… serve and intercept related resource plugin processing
@thescientist13

thescientist13 commented Sep 1, 2025

Copy link
Copy Markdown
Member

Took a little while with all the testing, but with this change in my branch, it has now been pretty stable again across Linux, Mac, and Windows.

If you want to try and extract that out (including my comments) it might be nice to help stabilize mainline for other open PRs, but either way I will be merging #1457 in a couples days as it is pretty much ready, just one little nice to have open item but not a blocker. Would also like to avoid any final merge conflicts on that PR as it has been in the works for a little while. 😮‍💨

@thescientist13 thescientist13 linked an issue Sep 1, 2025 that may be closed by this pull request
5 tasks
@KaiPrince

KaiPrince commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

yeah I saw that change and it confuses me because it shouldn't be necessary.

I encountered this issue before and thought I had found a fix. I can't reproduce it locally, so I've been testing in CI. KaiPrince#2

Anyhow if your PR is ready before I end up finding a fix just go ahead with it. If I discover the issue later and I'm able to commit a stable fix I'll make a follow-up PR. This draft PR is just a workbench for me to test in CI.

@KaiPrince

Copy link
Copy Markdown
Contributor Author

The hardest part about debugging this is it's so flaky. I thought I had fixed it previously, but turns out it can just pass for no reason. I'm hoping by the time I've figured this out, I've also managed to discover the cause of the flakiness and make it more deterministic.

image

@KaiPrince

Copy link
Copy Markdown
Contributor Author

Okay so this apparently fixed it. For reasons I have yet to understand

image

My current theory is it has something to do with the lifetime of the response.clone() object or the async gap.

@thescientist13
thescientist13 force-pushed the feature/issue-1247-standalone-markdown-plugin branch from 89adcea to c9abc33 Compare September 4, 2025 01:33
@thescientist13

thescientist13 commented Sep 4, 2025

Copy link
Copy Markdown
Member

Just a heads up I got my last little stretch item into #1457 so will probably merge by end of week, and builds still seem stable. (also I rebased against mainline so your history got a little messed up since it looks like you re-pointed to that branch)

That said, I wouldn't be opposed to continuing this effort if there are some race conditions / inconsistencies happening that we want to keep stabilizing. I know there could be some more consistency applied all around, especially with mergeResponse

Appreciate you looking into this!

@KaiPrince

Copy link
Copy Markdown
Contributor Author

Oh man, it's driving me crazy but I'm stumped. I have no explanation for why extracting a local const one line above fixes it in certain cases. I'm going to drop this and move on for now. If I'm hit with inspiration I'll revisit this.

@thescientist13

Copy link
Copy Markdown
Member

So even with CI being fairly stable, I even ran into this issue running greenwood build in a project as part of v0.33.0-alpha.8 testing, so I've decided to make a dedicated issue for this and captured it in this top level benchmarking / tracking issue - #970

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.

ensure Greenwood CLI and plugins are favoring idiomatic usages of async

2 participants