Skip to content

fix: use version of htmlrewriter which does not make use of asyncify, which looks to have a potential memory leak under high load #2707

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

fix: use version of htmlrewriter which does not make use of asyncify, which looks to have a potential memory leak under high load #2707

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d73accf
fix: use version of htmlrewriter which does not make use of asyncify,…
JakeChampion Nov 22, 2024
767b812
Merge branch 'main' into jake/htmlrewriter
serhalp Dec 9, 2024
b76dfb1
chore: vendor updated htmlrewriter
pieh Dec 9, 2024
2d968b4
fix: update remaining htmlrewriter import
pieh Dec 9, 2024
9ff1569
Merge branch 'main' into jake/htmlrewriter
pieh Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion deno.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
},
"imports": {
"@netlify/edge-functions": "https://edge.netlify.com/v1/index.ts"
}
},
"importMap": "./edge-runtime/vendor/import_map.json"
}
6 changes: 4 additions & 2 deletions edge-runtime/lib/response.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Context } from '@netlify/edge-functions'
import { HTMLRewriter } from '../vendor/deno.land/x/[email protected]/index.ts'
import { HTMLRewriter, init, type TextChunk } from 'https://deno.land/x/[email protected]/src/index.ts'
Copy link
Member

Choose a reason for hiding this comment

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

@JakeChampion I'm not super familiar with this setup, but it looks like all our deno imports use a relative path and should be included in this file 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be covered by 4a359c6


import { updateModifiedHeaders } from './headers.ts'
import type { StructuredLogger } from './logging.ts'
Expand All @@ -13,6 +13,8 @@ import {
relativizeURL,
} from './util.ts'

await init();

export interface FetchEventResult {
response: Response
waitUntil: Promise<any>
Expand Down Expand Up @@ -79,7 +81,7 @@ export const buildResponse = async ({

if (response.dataTransforms.length > 0) {
rewriter.on('script[id="__NEXT_DATA__"]', {
text(textChunk) {
text(textChunk: TextChunk) {
// Grab all the chunks in the Next data script tag
buffer += textChunk.text
if (textChunk.lastInTextNode) {
Expand Down
Loading