Skip to content

Headless chrome indexer error audit results fixes#3445

Merged
habdelra merged 18 commits intomainfrom
cs-9500-investigate-all-indexing-errors-that-we-see-when-starting-up
Oct 22, 2025
Merged

Headless chrome indexer error audit results fixes#3445
habdelra merged 18 commits intomainfrom
cs-9500-investigate-all-indexing-errors-that-we-see-when-starting-up

Conversation

@habdelra
Copy link
Copy Markdown
Contributor

@habdelra habdelra commented Oct 20, 2025

This PR fixes indexing issues discovered during the headless chrome indexer error audit:

  1. Indexing jobs need more lenient timeouts. When realms have no indexing errors or recoverable indexing errors the indexing is very fast, faster than our current indexer. However, when there are render errors that are not recoverable, the indexing is much much slower. in general the catalog realm needs much more time to index from a blank slate. Generally i'm seeing it take about 6 minutes to index from a blank slate. The experiments realm (2.5min), base realm (20s), and skills realm (5s) all index faster than before tho.
  2. The last known good html that appears in the error docs is getting confused for actual DOM when we try to render the serialized form of these errors. we strip this out from our errors before rendering them in the DOM so the browser doesn't get confused (this HTML is only meant for human consumption on errors--the prerenderer doesn't need it)
  3. the NotLoadedValue sentinel that we put in our deserialized state bucket for instances was leaking out when we try to render a card while the links are still inflight. I was able to clean this up for all teh field types except for LinksToMany. there is a deeper component stability issue here that we cant really address until the .value refactor. details have been left in the comments. however, this means that for LinksToMany, the NotLoaded sentinel will occasionally be exposed to card developers. I cleaned up a bunch of cards in our catalog realm where that happens. Codex was very very adamant that we don't try to go deeper with a fix here. it brought up lots of very good points about a super painful refactor that leaves us with lots of duplicated state management barring a .value refactor.
  4. Our glimmer scoped styles is actually causing issues for us due to the way it manipulates the DOM out from underneath glimmer. when we reuse a page and the render route is trying to tear down the DOM, glimmer gets mad that one of the DOM nodes that its trying to remove is no longer there (the <style> tag embedded in the card). I tried a bunch of approaches to this, but none were stable. so instead we detect this error and when we see it we clear the store cache and try again--this solves the issue.

Most of the rest of the issues are actually related to downloading modules from external sites and CORS violations due to our own custom headers. I broke that out into a separate ticket that i'll followup with shortly https://linear.app/cardstack/issue/CS-9562/remove-the-need-to-use-x-boxel-building-index-header.

Beyond that there are a handful of other errors that we don't see in the current indexer that I'm still tracking down. One nice thing tho is that there are also a bunch of errors in our current indexer around rendering cycles that are actually solved in the headless chrome indexer.

- waiting for linked fields to load only _after_ /render/html templates have been rendered
- more efficient render options
- improve error msg settling
- all node indexing test for headless chrome passing
…oveChild' on 'Node': The node to be removed is not a child of this node."

- scoped-css needs to respect the DOM tree that glimmer is managing when it moves the style tag around. otherwise this will blow up during re-renders in the /render route.
…ll-indexing-errors-that-we-see-when-starting-up
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 20, 2025

Host Test Results

    1 files  ±    0      1 suites  ±0   3h 2m 23s ⏱️ + 1h 53m 5s
1 523 tests +   98  1 509 ✅ +  102  13 💤 + 1  0 ❌  - 2  1 🔥  - 3 
4 397 runs  +2 960  4 350 ✅ +2 935  39 💤 +27  4 ❌  - 2  4 🔥 ±0 

For more details on these errors, see this check.

Results for commit f4ee460. ± Comparison against base commit 7486399.

This pull request removes 2 and adds 100 tests. Note that renamed tests count towards both.
Chrome ‑ Global error: Uncaught TypeError: Failed to fetch at http://localhost:7357/assets/chunk.8807cb9de65ae32da336.js, line 142924  After execution of test: Acceptance | Code patches tests: action bar disappears when "Cancel" button is clicked 
Chrome ‑ Global error: Uncaught TypeError: Failed to fetch at http://localhost:7357/assets/chunk.8807cb9de65ae32da336.js, line 142924  While executing test: Acceptance | Code patches tests: action bar disappears when "Cancel" button is clicked 
Chrome ‑ Acceptance | Code patches tests: LLM mode event controls auto-apply of code patches with timestamp checking
Chrome ‑ Acceptance | Code patches tests: action bar disappears when "Cancel" button is clicked
Chrome ‑ Acceptance | Code patches tests: automatic Accept All spinner appears in Act mode for multiple patches
Chrome ‑ Acceptance | Code patches tests: can create new files using the search/replace block
Chrome ‑ Acceptance | Code patches tests: can restore content of a patched file to its original state
Chrome ‑ Acceptance | Code patches tests: does not display the action bar when the streaming is cancelled
Chrome ‑ Acceptance | Code patches tests: loader reset happens when restoring patched executable files
Chrome ‑ Acceptance | Code patches tests: previously applied code patches show the correct applied state
Chrome ‑ Acceptance | Code patches tests: schema editor gets refreshed when code patches are executed on executable files
Chrome ‑ Acceptance | Code patches tests: when code patch is historic (user moved on to the next message), or it was applied, it will render the code (replace portion of the search/replace block) in a standard (non-diff) editor
…

♻️ This comment has been updated with latest results.

@habdelra habdelra marked this pull request as draft October 20, 2025 23:15
@habdelra
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@habdelra habdelra marked this pull request as ready for review October 21, 2025 14:30
@habdelra habdelra requested a review from a team October 21, 2025 14:31
// that a node it was tracking is no longer there. performing a new prerender
// capture with a cleared store/loader cache will workaround this issue.
[`Failed to execute 'removeChild' on 'Node'`, 'NotFoundError'],
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I’m surprised the style element is even present, I thought the AST transform would remove it from the component altogether

Copy link
Copy Markdown
Contributor Author

@habdelra habdelra Oct 21, 2025

Choose a reason for hiding this comment

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

agreed, I wonder if it’s the AST removing it that is what glimmer is mad about. i did confirm tho if I change the AST to leave the style tag in the card DOM it error goes away.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe there’s a way to have the transform happen earlier in the chain, before Glimmer has a hold on any elements at all. The way it runs on boxel-ui, for instance, is static. But I’m definitely out of my depth here.

…ll-indexing-errors-that-we-see-when-starting-up
@habdelra habdelra merged commit 9d4e0d4 into main Oct 22, 2025
110 of 116 checks passed
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