Skip to content

Commit

Permalink
fix: filter out middleware requests in logging (#64549)
Browse files Browse the repository at this point in the history
### What

When middleware.js is present, the logging is duplicated. We should
filter out the 1st middleware request and only log the actual one going
through request handler / renderer

Closes NEXT-3125
  • Loading branch information
huozhi authored Apr 16, 2024
1 parent 649a07c commit 2a1e170
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 10 deletions.
6 changes: 4 additions & 2 deletions packages/next/src/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1117,12 +1117,14 @@ export default class NextNodeServer extends BaseServer<
const { originalResponse } = normalizedRes

const reqStart = Date.now()
const isMiddlewareRequest = req.headers['x-middleware-invoke']

const reqCallback = () => {
// we don't log for non-route requests
const isRouteRequest = getRequestMeta(req).match
const routeMatch = getRequestMeta(req).match

const isRSC = isRSCRequestCheck(normalizedReq)
if (!isRouteRequest || isRSC) return
if (!routeMatch || isRSC || isMiddlewareRequest) return

const reqEnd = Date.now()
const fetchMetrics = normalizedReq.fetchMetrics || []
Expand Down
8 changes: 7 additions & 1 deletion test/e2e/app-dir/logging/app/page.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import Link from 'next/link'

export default function Page() {
return 'hello world'
return (
<>
<Link href={'/link'}>/link</Link>
</>
)
}
26 changes: 19 additions & 7 deletions test/e2e/app-dir/logging/fetch-logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import stripAnsi from 'strip-ansi'
import { retry } from 'next-test-utils'
import { nextTestSetup } from 'e2e-utils'

const cahceReasonRe = /Cache (missed|skipped) reason: /
const cacheReasonRegex = /Cache (missed|skipped) reason: /

interface ParsedLog {
method: string
Expand All @@ -17,13 +17,13 @@ interface ParsedLog {
function parseLogsFromCli(cliOutput: string) {
const logs = stripAnsi(cliOutput)
.split('\n')
.filter((log) => cahceReasonRe.test(log) || log.includes('GET'))
.filter((log) => cacheReasonRegex.test(log) || log.includes('GET'))

return logs.reduce<ParsedLog[]>((parsedLogs, log) => {
if (cahceReasonRe.test(log)) {
if (cacheReasonRegex.test(log)) {
// cache miss/skip reason
// Example of `log`: "│ │ Cache skipped reason: (cache: no-cache)"
const reasonSegment = log.split(cahceReasonRe, 3)[2].trim()
const reasonSegment = log.split(cacheReasonRegex, 3)[2].trim()
const reason = reasonSegment.slice(1, -1)
parsedLogs[parsedLogs.length - 1].cache = reason
} else {
Expand Down Expand Up @@ -155,15 +155,27 @@ describe('app-dir - logging', () => {
})
})

it('should exlucde Middleware invoked and _rsc requests', async () => {
it('should log each page request only once', async () => {
const outputIndex = next.cliOutput.length
await next.fetch('/')
await retry(() => {
const logsAfterRequest = stripAnsi(
next.cliOutput.slice(outputIndex)
)
// Only show `GET /` once
expect(logsAfterRequest.split('GET /').length).toBe(2)
})
})

it('should exlucde Middleware invoked and _rsc requests', async () => {
const outputIndex = next.cliOutput.length

const browser = await next.browser('/link')
await browser.elementByCss('a').click()
await browser.waitForElementByCss('h2')
const logs = stripAnsi(next.cliOutput.slice(outputIndex))
expect(logs).not.toContain('GET /_next/static')
expect(logs).not.toContain('GET /foo?_rsc')
expect(logs).not.toContain('/_next/static')
expect(logs).not.toContain('?_rsc')
})
}
} else {
Expand Down

1 comment on commit 2a1e170

@ijjk
Copy link
Member

@ijjk ijjk commented on 2a1e170 Apr 16, 2024

Choose a reason for hiding this comment

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

Stats from current release

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary v14.2.1 vercel/next.js canary Change
buildDuration 14.7s 27.5s ⚠️ +12.8s
buildDurationCached 9.2s 7.5s N/A
nodeModulesSize 199 MB 199 MB ⚠️ +76.9 kB
nextStartRea..uration (ms) 412ms 410ms N/A
Client Bundles (main, webpack)
vercel/next.js canary v14.2.1 vercel/next.js canary Change
2453-HASH.js gzip 31.4 kB 31.4 kB N/A
3304.HASH.js gzip 181 B 181 B
3f784ff6-HASH.js gzip 53.7 kB 53.7 kB
8299-HASH.js gzip 5.1 kB 5.1 kB N/A
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 242 B 242 B
main-HASH.js gzip 32.2 kB 29.7 kB N/A
webpack-HASH.js gzip 1.68 kB 1.68 kB N/A
Overall change 99.3 kB 99.3 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary v14.2.1 vercel/next.js canary Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary v14.2.1 vercel/next.js canary Change
_app-HASH.js gzip 196 B 197 B N/A
_error-HASH.js gzip 184 B 184 B
amp-HASH.js gzip 505 B 505 B
css-HASH.js gzip 324 B 325 B N/A
dynamic-HASH.js gzip 2.5 kB 2.5 kB N/A
edge-ssr-HASH.js gzip 258 B 258 B
head-HASH.js gzip 352 B 352 B
hooks-HASH.js gzip 370 B 371 B N/A
image-HASH.js gzip 4.27 kB 4.27 kB
index-HASH.js gzip 259 B 259 B
link-HASH.js gzip 2.67 kB 2.67 kB N/A
routerDirect..HASH.js gzip 314 B 312 B N/A
script-HASH.js gzip 386 B 386 B
withRouter-HASH.js gzip 309 B 309 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 6.63 kB 6.63 kB
Client Build Manifests
vercel/next.js canary v14.2.1 vercel/next.js canary Change
_buildManifest.js gzip 483 B 485 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary v14.2.1 vercel/next.js canary Change
index.html gzip 530 B 529 B N/A
link.html gzip 542 B 542 B
withRouter.html gzip 525 B 523 B N/A
Overall change 542 B 542 B
Edge SSR bundle Size
vercel/next.js canary v14.2.1 vercel/next.js canary Change
edge-ssr.js gzip 95.6 kB 94.4 kB N/A
page.js gzip 3.06 kB 3.05 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary v14.2.1 vercel/next.js canary Change
middleware-b..fest.js gzip 623 B 625 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 25.5 kB 25.5 kB N/A
edge-runtime..pack.js gzip 839 B 839 B
Overall change 839 B 839 B
Next Runtimes Overall increase ⚠️
vercel/next.js canary v14.2.1 vercel/next.js canary Change
app-page-exp...dev.js gzip 171 kB 171 kB N/A
app-page-exp..prod.js gzip 97.4 kB 97.5 kB N/A
app-page-tur..prod.js gzip 99.2 kB 99.2 kB N/A
app-page-tur..prod.js gzip 93.4 kB 93.5 kB N/A
app-page.run...dev.js gzip 144 kB 145 kB N/A
app-page.run..prod.js gzip 91.9 kB 92 kB N/A
app-route-ex...dev.js gzip 21.5 kB 21.5 kB N/A
app-route-ex..prod.js gzip 15.2 kB 15.2 kB N/A
app-route-tu..prod.js gzip 15.2 kB 15.2 kB N/A
app-route-tu..prod.js gzip 14.9 kB 14.9 kB N/A
app-route.ru...dev.js gzip 21.1 kB 21.1 kB N/A
app-route.ru..prod.js gzip 14.9 kB 14.9 kB N/A
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 22.5 kB 21.4 kB N/A
pages.runtim...dev.js gzip 23.1 kB 22.1 kB N/A
pages.runtim..prod.js gzip 22.5 kB 21.4 kB N/A
server.runti..prod.js gzip 51.3 kB 51.5 kB ⚠️ +253 B
Overall change 80.2 kB 80.5 kB ⚠️ +253 B
build cache Overall increase ⚠️
vercel/next.js canary v14.2.1 vercel/next.js canary Change
0.pack gzip 1.58 MB 1.6 MB ⚠️ +13.3 kB
index.pack gzip 107 kB 107 kB ⚠️ +832 B
Overall change 1.69 MB 1.7 MB ⚠️ +14.1 kB
Diff details
Diff for page.js

Diff too large to display

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 2453-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js
failed to diff
Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js
failed to diff
Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Please sign in to comment.