Skip to content

Add a catch to nativeStream.write(..) to avoid server crash. #7758

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

omerman
Copy link
Contributor

@omerman omerman commented Jul 27, 2025

What is it?

  • Bug
  • Infra

Description

There is an edge case as part of ssr rendering, where the server gotten an error after the headers are sent, the async nature of write(..) could trigger an error and if uncaught the process crashes.

Checklist

  • My code follows the developer guidelines of this project
  • I performed a self-review of my own code
  • I added a changeset with pnpm change
  • I made corresponding changes to the Qwik docs
  • I added new tests to cover the fix / functionality

@omerman omerman requested a review from a team as a code owner July 27, 2025 06:04
Copy link

changeset-bot bot commented Jul 27, 2025

🦋 Changeset detected

Latest commit: 7f5fc29

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@builder.io/qwik Patch
eslint-plugin-qwik Patch
@builder.io/qwik-city Patch
create-qwik Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@gioboa gioboa left a comment

Choose a reason for hiding this comment

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

Thanks @itsthesteve for your help
It looks great to me.
@wmertens thoughts?

@omerman
Copy link
Contributor Author

omerman commented Jul 27, 2025

Thanks @itsthesteve for your help It looks great to me. @wmertens thoughts?

Hey I think I see I still have issues here there are many more .write(..) places I'll try to think of a better approach.

@omerman
Copy link
Contributor Author

omerman commented Jul 27, 2025

On second thought, I may be over my head here.
I could keep only the specific place I've modified to prevent the server from crashing, but there is a long list of stream.write(..) without await/catch.

Let me know what you think we should do here @gioboa @wmertens

packages/qwik/src/core/render/ssr/render-ssr.ts:
  240  ) => {
  241:   stream.write(FLUSH_COMMENT);
  242    const generator = node.props.children;

  246        write(chunk) {
  247:         stream.write(chunk);
  248:         stream.write(FLUSH_COMMENT);
  249        },

  259      await processData(chunk, rCtx, ssrCtx, stream, flags, undefined);
  260:     stream.write(FLUSH_COMMENT);
  261    }

  290    virtualComment += '-->';
  291:   stream.write(virtualComment);
  292  

  294    if (html) {
  295:     stream.write(html);
  296:     stream.write(CLOSE_VIRTUAL);
  297      return;

  308      if (!isSlot && !beforeClose) {
  309:       stream.write(CLOSE_VIRTUAL);
  310        return;

  330      return maybeThen(promise, () => {
  331:       stream.write(CLOSE_VIRTUAL);
  332      });

  370  ) => {
  371:   stream.write('<' + tagName + renderAttributes(attributes) + '>');
  372    const empty = !!emptyElements[tagName];

  379    if (innerHTML != null) {
  380:     stream.write(innerHTML);
  381    }
  382:   stream.write(`</${tagName}>`);
  383  };

  774      openingElement += '>';
  775:     stream.write(openingElement);
  776  

  781      if (htmlStr != null) {
  782:       stream.write(String(htmlStr));
  783:       stream.write(`</${tagName}>`);
  784        return;

  804        if (!beforeClose) {
  805:         stream.write(`</${tagName}>`);
  806          return;

  810        return maybeThen(beforeClose(stream), () => {
  811:         stream.write(`</${tagName}>`);
  812        });

  839    if (tagName === SSRRaw) {
  840:     stream.write((node as JSXNodeInternal<typeof SSRRaw>).props.data);
  841      return;

  886    if (isString(node) || typeof node === 'number') {
  887:     stream.write(escapeHtml(String(node)));
  888    } else if (isJSXNode(node)) {

  908          }
  909:         stream.write(`<!--t=${id}-->`);
  910          processData(value, rCtx, ssrCtx, stream, flags, beforeClose);
  911:         stream.write(`<!---->`);
  912          return;

  916      }
  917:     stream.write(escapeHtml(jsxToString(value)));
  918      return;
  919    } else if (isPromise(node)) {
  920:     stream.write(FLUSH_COMMENT);
  921      return node.then((node) => processData(node, rCtx, ssrCtx, stream, flags, beforeClose));

  957              if (currentIndex === index) {
  958:               stream.write(chunk);
  959              } else {

  970          if (buffers.length > currentIndex) {
  971:           buffers[currentIndex].forEach((chunk) => stream.write(chunk));
  972          }

packages/qwik/src/server/render.ts:
   52      if (buffer) {
   53:       nativeStream.write(buffer).catch((e) => {
   54          console.error(`Could not write buffer: ${buffer.slice(0, 200)}...`);

  106    if (containerTagName === 'html') {
  107:     stream.write(DOCTYPE);
  108    } else {
  109:     stream.write('<!--cq-->');
  110    }

  219    if (containerTagName !== 'html') {
  220:     stream.write('<!--/cq-->');
  221    }


packages/qwik-city/src/middleware/request-handler/request-event.ts:
  114        const writer = writableStream.getWriter();
  115:       writer.write(typeof body === 'string' ? encoder.encode(body) : body);
  116        writer.close();

packages/qwik-city/src/middleware/request-handler/resolve-request-handlers.ts:
  347              }
  348:             await stream.write(encoder.encode(`${message}\n`));
  349            }

  511          // write the already completed html to the stream
  512:         await stream.write((result as any as RenderToStringResult).html);
  513        }

  589    const data = await qwikSerializer._serializeData(qData, true);
  590:   writer.write(encoder.encode(data));
  591    requestEv.sharedMap.set('qData', qData);

packages/qwik-city/src/middleware/request-handler/user-response.ts:
  119                const writer = stream.getWriter();
  120:               await writer.write(encoder.encode(getErrorHtml(500, 'Internal Server Error')));
  121                await writer.close();

packages/qwik-city/src/static/not-found.ts:
  23          const writer = sys.createWriteStream(filePath);
  24:         writer.write(html);
  25          writer.end(resolve);

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