Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server crashes due to multiple runtime errors in Astro component (Astro + React) #10367

Closed
1 task
frendyguo opened this issue Mar 8, 2024 · 1 comment · Fixed by #10448 or #10454
Closed
1 task
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@frendyguo
Copy link

Astro Info

Astro                    v4.4.15
Node                     v18.19.0
System                   macOS (x64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             @astrojs/react

If this issue only occurs in one browser, which browser is a problem?

All browsers

Describe the Bug

When rendering an Astro component (index.astro), if there is an error in a React component preceded by another runtime error, it will result in a server crash.

Example:

---
// index.astro
import ReactComp from '../components/react-comp';

const foo = { bar: null } as unknown as { bar: { baz: string[] } };
---

<!-- React component has to error first -->
<ReactComp foo={foo} />
{ foo.bar.baz.length > 0 && (
  <div />
)}

Steps to reproduce

  1. Run npm run build
  2. Run npm run preview
  3. Go to localhost
  4. Go to / page, server crashes.

What's the expected result?

Should not crash the server no matter how the error happen.

Link to Minimal Reproducible Example

https://github.com/frendyguo/astro-react-runtime-error-crash

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Mar 8, 2024
@bluwy bluwy added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Mar 8, 2024
@matthewp matthewp self-assigned this Mar 8, 2024
@bluwy bluwy added - P4: important Violate documented behavior or significantly impacts performance (priority) and removed - P3: minor bug An edge case that only affects very specific usage (priority) labels Mar 8, 2024
@bluwy
Copy link
Member

bluwy commented Mar 11, 2024

I looked into this, and it seems trickier to fix than expected. For example, the generated code from the repro is this (prettified so easier to read):

const $$Index = createComponent(
  async ($$result, $$props, $$slots) => {
    const Astro2 = $$result.createAstro($$Astro, $$props, $$slots)
    Astro2.self = $$Index
    const foo = { bar: null }
    return renderTemplate`<!-- React component has to error first -->
      ${renderComponent($$result, 'ReactComp', ReactComp, { foo: foo })}
      ${foo.bar.baz.length > 0 && renderTemplate`${maybeRenderHead()}<div></div>`}`
  },
  '/Users/.../src/pages/index.astro',
  void 0
)

The first renderComponent call would error, but it only returns a promise that rejects. The second foo.bar.baz.length > 0 && ... also errors but returns synchronously. So when rendering this Astro component, the second error will immediately return synchronously so the server responds with a 500 status. However, that leaves the first promise that rejects "hanging" or unhandled.

It's tricky the capture the first promise. Some ideas would be:

  1. renderComponent (or any async APIs) could tell $$result to wait for the promise?
  2. renderComponent (or any async APIs) could tell $$result to wait unhandledRejection event?
  3. Make sure compiler injects await before renderComponent?

no3 doesn't feel right, but it's kinda the easiest fix. If we want something more robust, no1 or no2 would be it, but they don't look like the cleanest fix. I'm leaning on no3 currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
3 participants