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

corsify creating unused readable stream #261

Open
shsteimer opened this issue Aug 30, 2024 · 2 comments · May be fixed by #268
Open

corsify creating unused readable stream #261

shsteimer opened this issue Aug 30, 2024 · 2 comments · May be fixed by #268

Comments

@shsteimer
Copy link

Describe the Issue

When running locally with dev tools open, an error is produced that states:

"A ReadableStream branch was created but never consumed. Such branches can be created, for instance, by calling the tee() method on a ReadableStream, or by calling the clone() method on a Request or Response object. If a branch is created but never consumed, it can force the runtime to buffer the entire body of the stream in memory, which may cause the Worker to exceed its memory limit and be terminated. To avoid this, ensure that all branches created are consumed."

This seems to have been introduced in 5.0.17 with the change to clone the response 74f63ab#diff-a1624121bc1fee18f8a5c2b2f907f01ba9c56a50068007e4c5da2911e91ea6df

Example Router Code

Please provide the itty-router code related to the issue. If possible, create a minimal, reproducible example.

import { Router, cors } from 'itty-router';

const { preflight, corsify } = cors({
	origin: (origin) => {
		const patterns = ['.allowedurl.com', 'localhost:3000'];
		if (origin && patterns.some((p) => origin.endsWith(p))) {
			return origin;
		}

		return undefined;
	},
	allowMethods: ['GET', 'POST'],
	maxAge: 84600,
});

const router = Router({
	before: [preflight],
	finally: [corsify],
});

//register some routes
router.get('/api/myRoute', anAsyncRouteFunction);

Request Details

appears to happen on every request so I'm not sure it matters, but:

  • Method: GET
  • URL: /api/myRoute?someParam=someValue
  • Request Body: If applicable, include the request body.
  • Request Headers: If applicable, include the request headers.

Steps to Reproduce

Steps to reproduce the behavior:

  1. create a worker project using Router and corsify
  2. run wrangler dev
  3. type 'd' to open dev tools
  4. send a request to your worker
  5. see in the dev tools console that the error is produced

Expected Behavior

It's not clear to me how big a problem this is in reality, but it is definitely a daunting error message, so should be avoided if at all possible.

Actual Behavior

A clear and concise description of what actually happens. Include any error messages or unexpected responses.

Environment (please complete the following information):

  • Environment: Cloudflare Workers
  • itty-router Version: 5.0.18 (downgrading to 5.0.16 removes the issue)

Additional Context

issues was also reported at kwhitley/itty.dev#26

@istarkov
Copy link
Contributor

Moreover it does not prevent from the error can’t modify immutable headers.
response.clone() used is wrong

@leongrdic
Copy link

I'm having the same issue using the latest version.

jacwright added a commit to jacwright/itty-router that referenced this issue Jan 16, 2025
Fixes kwhitley#261 by creating a new response with mutable headers and avoiding duplicating the body. The correct fix for what kwhitley@74f63ab#diff-a1624121bc1fee18f8a5c2b2f907f01ba9c56a50068007e4c5da2911e91ea6df was trying to achieve.
@jacwright jacwright linked a pull request Jan 16, 2025 that will close this issue
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 a pull request may close this issue.

3 participants