feat: add logs:edge-functions command and historical log support#7944
Open
feat: add logs:edge-functions command and historical log support#7944
Conversation
📊 Benchmark resultsComparing with 61149eb
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Add edge function log streaming and historical log fetching for both function and edge function logs. The deploy command output now shows CLI hints for accessing logs when functions are deployed. - Add `logs:edge-functions` command with WebSocket streaming and historical mode via `--from`/`--to` - Add `--from`/`--to` date options to `logs:function` for historical log fetching via analytics API - Add `--deploy-id` option to `logs:function` and `logs:edge-functions` to look up functions from a specific deploy - Accept function ID (not just name) in `logs:function` - Show function/edge-function CLI log hints in deploy output - Create shared `log-api.ts` utility for date parsing, API fetching, and log formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The deploy API response doesn't include an edge_functions_count field. Thread the edgeFunctionsCount from deploySite's local hashing step through to the deploy output so edge function log hints display correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…g sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…g sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add proper type assertions to eliminate unsafe-any and non-null assertion lint errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
biilmann
force-pushed
the
logs-for-agents
branch
from
1019b05 to
bd52070
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new logs:edge-functions CLI subcommand that supports live WebSocket streaming and historical REST fetches for edge function logs. Extends logs:function with --deploy-id, --from, and --to to support deploy-scoped and time-bound queries and broadened function selection (by name or ID). Introduces a new log-api utility module for date parsing, analytics URL building, fetching, and formatting historical logs. Modifies deploy flow to surface deployedFunctions and hasEdgeFunctions and include deployed_functions in JSON output. Adds command wiring, documentation updates, and integration tests for edge-function and function log flows. Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
tests/integration/commands/logs/edge-functions.test.ts (1)
199-216: URL substring match is less precise thannew URL().hostnameused infunctions.test.ts.The
url.includes('analytics.services.netlify.com')check can match the hostname appearing anywhere in the URL string (e.g., as a query parameter). The companionfunctions.test.tsusesnew URL(url).hostnamefor the equivalent mock routing, which is more precise. Aligning on the hostname approach would be consistent.♻️ Proposed fix (align with functions.test.ts approach)
- const spyFetch = vi.fn().mockImplementation((url: string) => { - if (url.includes('analytics.services.netlify.com')) { + const spyFetch = vi.fn().mockImplementation((url: string) => { + const parsedUrl = new URL(url) + if (parsedUrl.hostname === 'analytics.services.netlify.com') {- const analyticsCall = spyFetch.mock.calls.find((args: string[]) => - args[0].includes('analytics.services.netlify.com'), + const analyticsCall = spyFetch.mock.calls.find((args: string[]) => { + const parsedUrl = new URL(args[0]) + return parsedUrl.hostname === 'analytics.services.netlify.com' + } )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/commands/logs/edge-functions.test.ts` around lines 199 - 216, The host-matching in the fetch mock is too broad; update the mockImplementation in spyFetch to parse the input URL and compare the hostname exactly (e.g., use new URL(url).hostname === 'analytics.services.netlify.com') instead of url.includes(...), keeping the same return behavior for that host; ensure you reference the existing spyFetch/mockImplementation and originalFetch variables and keep global.fetch assignment and subsequent assertions (spyWebsocket, program.parseAsync, spyFetch.mock.calls) unchanged.
🧹 Nitpick comments (4)
src/commands/logs/log-api.ts (1)
12-32: Add a request timeout to prevent hangs.Consider aborting the request after a reasonable timeout to avoid indefinitely blocking the CLI on slow or stalled networks.
♻️ Suggested update with AbortController
export async function fetchHistoricalLogs({ url, accessToken, }: { url: string accessToken: string }): Promise<unknown> { - const response = await fetch(url, { - method: 'GET', - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }) + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 30_000) + const response = await fetch(url, { + method: 'GET', + headers: { + Authorization: `Bearer ${accessToken}`, + }, + signal: controller.signal, + }) - if (!response.ok) { - const errorData = (await response.json().catch(() => ({}))) as { error?: string } - throw new Error(errorData.error ?? `HTTP ${response.status.toString()}: ${response.statusText}`) - } + try { + if (!response.ok) { + const errorData = (await response.json().catch(() => ({}))) as { error?: string } + throw new Error(errorData.error ?? `HTTP ${response.status.toString()}: ${response.statusText}`) + } + } finally { + clearTimeout(timeout) + } return response.json() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/logs/log-api.ts` around lines 12 - 32, The fetchHistoricalLogs function can hang on slow networks; create an AbortController, pass controller.signal into fetch, and start a timeout (e.g., 10s) with setTimeout that calls controller.abort() to cancel the request; after fetch completes clearTimeout to avoid leaks and handle AbortError by throwing a clear timeout-specific Error (or rethrow) so the CLI doesn't hang; update fetchHistoricalLogs to use the controller/signal and proper cleanup around the existing response.ok/error handling and response.json call.src/commands/logs/edge-functions.ts (1)
27-27: Redundant.toString()—CLI_LOG_LEVEL_CHOICES_STRINGis an array.
CLI_LOG_LEVEL_CHOICES_STRINGisLOG_LEVELS_LIST.map(...)— an array. Template literals coerce arrays to strings automatically (same as calling.toString()), so the explicit call is redundant.functions.tsomits it consistently.♻️ Proposed fix
- log(`Invalid log level. Choices are:${CLI_LOG_LEVEL_CHOICES_STRING.toString()}`) + log(`Invalid log level. Choices are:${CLI_LOG_LEVEL_CHOICES_STRING}`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/logs/edge-functions.ts` at line 27, The template literal in the log call redundantly calls .toString() on the array CLI_LOG_LEVEL_CHOICES_STRING; remove the explicit .toString() so the line reads log(`Invalid log level. Choices are:${CLI_LOG_LEVEL_CHOICES_STRING}`) and mirror the style used in functions.ts, ensuring you update the usage in edge-functions.ts where the log(...) invocation occurs.src/commands/logs/functions.ts (1)
34-36: MissingsiteIdnull guard — inconsistent withedge-functions.ts.
edge-functions.tsexplicitly checksif (!siteId)and exits with a helpful message.functions.tsusessiteId!non-null assertions at lines 46/50, so if the project isn't linked, the URL at line 85 would containundefinedas a path segment.🛡️ Proposed guard (mirrors edge-functions.ts)
const { id: siteId } = site + + if (!siteId) { + log('You must link a project before attempting to view function logs') + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/logs/functions.ts` around lines 34 - 36, Add the same null-guard used in edge-functions.ts to functions.ts: check the extracted siteId (from const { id: siteId } = site) before using it, and if it's falsy call command.out.error with a helpful message and exit/return early so code never uses siteId!; update any places currently using siteId! (e.g., the URL construction around line ~85) to rely on the guarded, non-null siteId instead.src/commands/deploy/deploy.ts (1)
815-818: Update CLI hint placeholder to reflect ID support.The command now accepts function IDs, so the hint should not imply name-only usage.
Proposed tweak
- 'Function CLI': `netlify logs:function --deploy-id ${results.deployId} <function-name>`, + 'Function CLI': `netlify logs:function --deploy-id ${results.deployId} <function-name-or-id>`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deploy/deploy.ts` around lines 815 - 818, The CLI hint in the functionLogsData object currently suggests a name-only placeholder ("<function-name>") even though the command accepts function IDs; update the 'Function CLI' string to indicate both ID and name are supported (e.g., use "<function-id|name>" or "<function-id or function-name>") so users aren’t misled. Locate the functionLogsData constant and change the template string that builds `netlify logs:function --deploy-id ${results.deployId} <function-name>` to use the combined placeholder while keeping the --deploy-id ${results.deployId} interpolation intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/commands/logs.md`:
- Line 108: Replace the vision-based phrase "look up" in the `deploy-id` flag
description with a non-vision term; update the line that reads "`deploy-id`
(*string*) - Deploy ID to look up the function from" to use "find" or "retrieve"
(e.g., "Deploy ID to find the function" or "Deploy ID to retrieve the function")
so Vale's accessibility rule base.accessibilityVision is satisfied.
- Line 102: Update the user-facing argument description that currently uses the
raw token functionName so Vale stops flagging it; either enclose the token in
backticks (`functionName - Name or ID of the function to stream logs for`) or
reword it to plain English (e.g., "function name - Name or ID of the function to
stream logs for") in the logs command documentation entry where functionName
appears.
In `@src/commands/logs/functions.ts`:
- Around line 83-85: The URL currently interpolates branch and
resolvedFunctionName directly into the path (see variables branch and
resolvedFunctionName and the url constant), which breaks for branch names
containing slashes; update the URL construction to apply encodeURIComponent to
each path segment (encode branch and resolvedFunctionName) before interpolation
so the path segments are safely encoded while leaving query parameters
(fromMs/toMs) unchanged.
In `@src/commands/logs/log-api.ts`:
- Around line 56-71: printHistoricalLogs currently compares entry.level
lowercased against levelsToPrint as-is, so uppercase filters like "ERROR" will
miss matches; normalize the incoming levelsToPrint once (e.g., map to lowercase
and optionally trim, or build a Set of lowercased values) at the start of
printHistoricalLogs and then compare the lowercased entry.level against that
normalized collection (use the existing local variable level for the entry
comparison) to avoid case-sensitive mismatches.
In `@tests/integration/commands/logs/functions.test.ts`:
- Line 205: The three new tests (including the one with title 'should find
function by ID') use an empty destructured parameter "async ({}) =>", which
triggers Biome's noEmptyPattern lint error; edit each test's declaration (all
tests that currently use async ({}) =>) and replace the empty destructuring with
an empty parameter list "async () =>" so the test functions are declared as
async () => instead of async ({}) => (look for the test titles such as 'should
find function by ID' and the two other newly added tests that use the same
pattern).
---
Duplicate comments:
In `@tests/integration/commands/logs/edge-functions.test.ts`:
- Around line 199-216: The host-matching in the fetch mock is too broad; update
the mockImplementation in spyFetch to parse the input URL and compare the
hostname exactly (e.g., use new URL(url).hostname ===
'analytics.services.netlify.com') instead of url.includes(...), keeping the same
return behavior for that host; ensure you reference the existing
spyFetch/mockImplementation and originalFetch variables and keep global.fetch
assignment and subsequent assertions (spyWebsocket, program.parseAsync,
spyFetch.mock.calls) unchanged.
---
Nitpick comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 815-818: The CLI hint in the functionLogsData object currently
suggests a name-only placeholder ("<function-name>") even though the command
accepts function IDs; update the 'Function CLI' string to indicate both ID and
name are supported (e.g., use "<function-id|name>" or "<function-id or
function-name>") so users aren’t misled. Locate the functionLogsData constant
and change the template string that builds `netlify logs:function --deploy-id
${results.deployId} <function-name>` to use the combined placeholder while
keeping the --deploy-id ${results.deployId} interpolation intact.
In `@src/commands/logs/edge-functions.ts`:
- Line 27: The template literal in the log call redundantly calls .toString() on
the array CLI_LOG_LEVEL_CHOICES_STRING; remove the explicit .toString() so the
line reads log(`Invalid log level. Choices are:${CLI_LOG_LEVEL_CHOICES_STRING}`)
and mirror the style used in functions.ts, ensuring you update the usage in
edge-functions.ts where the log(...) invocation occurs.
In `@src/commands/logs/functions.ts`:
- Around line 34-36: Add the same null-guard used in edge-functions.ts to
functions.ts: check the extracted siteId (from const { id: siteId } = site)
before using it, and if it's falsy call command.out.error with a helpful message
and exit/return early so code never uses siteId!; update any places currently
using siteId! (e.g., the URL construction around line ~85) to rely on the
guarded, non-null siteId instead.
In `@src/commands/logs/log-api.ts`:
- Around line 12-32: The fetchHistoricalLogs function can hang on slow networks;
create an AbortController, pass controller.signal into fetch, and start a
timeout (e.g., 10s) with setTimeout that calls controller.abort() to cancel the
request; after fetch completes clearTimeout to avoid leaks and handle AbortError
by throwing a clear timeout-specific Error (or rethrow) so the CLI doesn't hang;
update fetchHistoricalLogs to use the controller/signal and proper cleanup
around the existing response.ok/error handling and response.json call.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (29)
docs/commands/agents.mddocs/commands/api.mddocs/commands/blobs.mddocs/commands/build.mddocs/commands/clone.mddocs/commands/completion.mddocs/commands/db.mddocs/commands/dev.mddocs/commands/env.mddocs/commands/functions.mddocs/commands/init.mddocs/commands/link.mddocs/commands/login.mddocs/commands/logs.mddocs/commands/open.mddocs/commands/recipes.mddocs/commands/sites.mddocs/commands/status.mddocs/commands/unlink.mddocs/commands/watch.mddocs/index.mdsrc/commands/deploy/deploy.tssrc/commands/logs/edge-functions.tssrc/commands/logs/functions.tssrc/commands/logs/index.tssrc/commands/logs/log-api.tssrc/utils/deploy/deploy-site.tstests/integration/commands/logs/edge-functions.test.tstests/integration/commands/logs/functions.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docs/commands/logs.md (2)
102-102:⚠️ Potential issue | 🟡 MinorWrap
functionNamein code formatting (or rephrase).
functionNameis still flagged by Vale spellcheck in user-facing docs. Use backticks or plain words (function name) to avoid the warning.✏️ Suggested fix
-- functionName - Name or ID of the function to stream logs for +- `functionName` - Name or ID of the function to stream logs for🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/commands/logs.md` at line 102, Update the docs/commands/logs.md line that currently shows "functionName - Name or ID of the function to stream logs for" by wrapping functionName in inline code backticks or rewording it to plain words (e.g., `functionName` or "function name") so Vale spellcheck no longer flags it; locate the occurrence of the identifier "functionName" in that file and replace it with the chosen formatted/rephrased version.
106-106:⚠️ Potential issue | 🟡 MinorReplace vision-based phrase “look up” in flag description.
This wording is still flagged by Vale accessibility rules; switch to “find” or “retrieve”.
✏️ Suggested fix
-- `deploy-id` (*string*) - Deploy ID to look up the function from +- `deploy-id` (*string*) - Deploy ID to find the function from🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/commands/logs.md` at line 106, The flag description for `deploy-id` uses the vision-based phrase "look up"; update the description in the `deploy-id` flag entry to use an alternative such as "find" or "retrieve" (e.g., change "Deploy ID to look up the function from" to "Deploy ID to find the function" or "Deploy ID to retrieve the function") so it no longer uses vision-based language.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/commands/logs.md`:
- Line 102: Update the docs/commands/logs.md line that currently shows
"functionName - Name or ID of the function to stream logs for" by wrapping
functionName in inline code backticks or rewording it to plain words (e.g.,
`functionName` or "function name") so Vale spellcheck no longer flags it; locate
the occurrence of the identifier "functionName" in that file and replace it with
the chosen formatted/rephrased version.
- Line 106: The flag description for `deploy-id` uses the vision-based phrase
"look up"; update the description in the `deploy-id` flag entry to use an
alternative such as "find" or "retrieve" (e.g., change "Deploy ID to look up the
function from" to "Deploy ID to find the function" or "Deploy ID to retrieve the
function") so it no longer uses vision-based language.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/commands/logs.md
| | Subcommand | description | | ||
| |:--------------------------- |:-----| | ||
| | [`logs:deploy`](/commands/logs#logsdeploy) | Stream the logs of deploys currently being built to the console | | ||
| | [`logs:edge-functions`](/commands/logs#logsedge-functions) | Stream netlify edge function logs to the console | |
Member
There was a problem hiding this comment.
I understand why this is plural and logs:function is singular, but it still itches! 😖
In the future, I think we could rename logs:function to logs:functions and start accepting multiple function names, since there's nothing stopping us from listening to different streams and interleaving them, just like we do with edge functions.
src/commands/deploy/deploy.ts
Outdated
| const functionLogsData: Record<string, string> = { | ||
| 'Function logs': terminalLink(results.functionLogsUrl, results.functionLogsUrl, { fallback: false }), | ||
| 'Edge function Logs': terminalLink(results.edgeFunctionLogsUrl, results.edgeFunctionLogsUrl, { fallback: false }), | ||
| 'Function CLI': `netlify logs:function --deploy-id ${results.deployId} <function-name>`, |
Member
There was a problem hiding this comment.
"Function CLI" feels a bit weird? How about:
- Functions logs
- Web: https://(...)
- CLI: `netlify logs:function (...)`
- Edge Functions logs
- Web: https://(...)
- CLI: `netlify logs:edge-functions (...)`
| } | ||
| } | ||
|
|
||
| const ws = getWebSocket('wss://socketeer.services.netlify.com/edge-function/logs') |
Member
There was a problem hiding this comment.
Side note: I hate that we're exposing this. We should set up a customer-facing domain name for this.
| return ms | ||
| } | ||
|
|
||
| export async function fetchHistoricalLogs({ |
Member
There was a problem hiding this comment.
This method doesn't really have anything to do with fetching logs. It's just making a fetch call and returning the response. I would recommend either getting rid of it and making the call directly, or moving the URL generation over here. I would recommend the latter, since we're currently constructing the analytics endpoint in different places and we could centralise that.
Centralize analytics URL generation into log-api.ts, add siteId null guard in functions.ts, encode URL path segments, normalize log level filtering, fix lint issues in tests, and update deploy output labels. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/integration/commands/logs/edge-functions.test.ts (1)
140-163: Cover the--deploy-id+--fromcombination.These tests exercise
--deploy-idand historical fetching separately, but not together. That leaves the current historical no-op on--deploy-iduntested, so the bug insrc/commands/logs/edge-functions.tswould still pass this suite.Also applies to: 189-225
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/commands/logs/edge-functions.test.ts` around lines 140 - 163, Add a new integration test that combines --deploy-id with --from to assert the CLI treats deploy-id as non-historical (i.e., still sends deploy_id in the websocket payload and does not attempt historical fetch); reuse the existing test "should use deploy ID from --deploy-id option when provided" as a template in tests/integration/commands/logs/edge-functions.test.ts, call program.parseAsync with both '--deploy-id','my-deploy-id' and '--from','<timestamp>', trigger the websocket open callback, inspect spySend.mock.calls to JSON.parse the sent body and assert body.deploy_id === 'my-deploy-id' and/or that no historical-mode flag or historical fetch was initiated; add the same coverage around the other test block referenced (lines ~189-225) to ensure both cases are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/logs/edge-functions.ts`:
- Line 13: The CLI's historical code path ignores the deployId option (deployId
/ options.deployId) when --from is used, causing --deploy-id to be a no-op;
update the historical request builder/handler (the code branch that handles
options.from / "historical mode") to include deployId as a filter/query
parameter when constructing the historical analytics/logs URL/request (or
alternatively, detect the unsupported combination and fail fast with a clear
error), and apply the same fix to the related code around the other occurrences
noted (the block covering lines 32-39) so deploy scoping is respected or
rejected consistently.
In `@src/commands/logs/functions.ts`:
- Around line 50-57: When options.deployId is provided the code uses
getSiteDeploy() to load a deploy into the local variable deploy but later still
computes the branch from siteInfo.build_settings?.repo_branch ?? 'main', causing
historical lookups to use the wrong (default) branch; update the deploy-id path
to derive the branch from the deploy object (e.g.,
deploy.build_settings?.repo_branch or deploy.branch) and pass that branch into
any historical analytics/fetch code, or else detect the absence of branch info
on the deploy and reject deploy-scoped historical queries; change logic in the
getSiteDeploy branch where functions is set (and mirror the same fix for the
other occurrence around the 85-92 area) so historical lookups use the
deploy-derived branch instead of siteInfo.build_settings?.repo_branch.
In `@src/commands/logs/log-api.ts`:
- Around line 86-101: printHistoricalLogs currently logs "No logs found..." only
when the API returned zero entries, but if entries exist and all are filtered
out by levelsToPrint it exits silently; change the logic in printHistoricalLogs
to apply the level filter to entries first (use normalizedLevels and
entry.level) or track whether any entry was printed (use a printedCount flag)
and after the loop call log('No logs found for the specified time range') when
no entries matched; refer to symbols entries, normalizedLevels, formatLogEntry,
and log to locate the code to modify.
---
Nitpick comments:
In `@tests/integration/commands/logs/edge-functions.test.ts`:
- Around line 140-163: Add a new integration test that combines --deploy-id with
--from to assert the CLI treats deploy-id as non-historical (i.e., still sends
deploy_id in the websocket payload and does not attempt historical fetch); reuse
the existing test "should use deploy ID from --deploy-id option when provided"
as a template in tests/integration/commands/logs/edge-functions.test.ts, call
program.parseAsync with both '--deploy-id','my-deploy-id' and
'--from','<timestamp>', trigger the websocket open callback, inspect
spySend.mock.calls to JSON.parse the sent body and assert body.deploy_id ===
'my-deploy-id' and/or that no historical-mode flag or historical fetch was
initiated; add the same coverage around the other test block referenced (lines
~189-225) to ensure both cases are covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec1b0dd8-b3ea-42a9-95c1-87f890fe6a13
📒 Files selected for processing (7)
docs/commands/logs.mdsrc/commands/deploy/deploy.tssrc/commands/logs/edge-functions.tssrc/commands/logs/functions.tssrc/commands/logs/log-api.tstests/integration/commands/logs/edge-functions.test.tstests/integration/commands/logs/functions.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/integration/commands/logs/functions.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/commands/logs.md
| import { getName } from './build.js' | ||
|
|
||
| export const logsEdgeFunction = async (options: OptionValues, command: BaseCommand) => { | ||
| let deployId = options.deployId as string | undefined |
There was a problem hiding this comment.
--deploy-id becomes a no-op in historical mode.
deployId is captured from the CLI options, but the --from path always builds a site-wide analytics URL and returns immediately. netlify logs:edge-functions --deploy-id <id> --from ... therefore ignores the requested deploy and can show unrelated logs. Either include deploy scoping in the historical request or fail fast for this flag combination.
Also applies to: 32-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/logs/edge-functions.ts` at line 13, The CLI's historical code
path ignores the deployId option (deployId / options.deployId) when --from is
used, causing --deploy-id to be a no-op; update the historical request
builder/handler (the code branch that handles options.from / "historical mode")
to include deployId as a filter/query parameter when constructing the historical
analytics/logs URL/request (or alternatively, detect the unsupported combination
and fail fast with a clear error), and apply the same fix to the related code
around the other occurrences noted (the block covering lines 32-39) so deploy
scoping is respected or rejected consistently.
Comment on lines
+50
to
+57
| if (options.deployId) { | ||
| const deploy = (await client.getSiteDeploy({ siteId: siteId, deployId: options.deployId })) as any | ||
| functions = deploy.available_functions ?? [] | ||
| } else { | ||
| // TODO: Update type once the open api spec is updated https://open-api.netlify.com/#tag/function/operation/searchSiteFunctions | ||
| const result = (await client.searchSiteFunctions({ siteId: siteId })) as any | ||
| functions = result.functions ?? [] | ||
| } |
There was a problem hiding this comment.
Keep deploy-scoped historical lookups on the deploy's branch.
The --deploy-id path switches function discovery over to getSiteDeploy(), but the historical fetch still hard-codes siteInfo.build_settings?.repo_branch ?? 'main'. For branch deploys and deploy previews, that will query the wrong analytics path and return default-branch logs instead of logs for the selected deploy. Retain the deploy response and derive the branch from it, or reject deploy-scoped historical queries until that data is available.
Also applies to: 85-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/logs/functions.ts` around lines 50 - 57, When options.deployId
is provided the code uses getSiteDeploy() to load a deploy into the local
variable deploy but later still computes the branch from
siteInfo.build_settings?.repo_branch ?? 'main', causing historical lookups to
use the wrong (default) branch; update the deploy-id path to derive the branch
from the deploy object (e.g., deploy.build_settings?.repo_branch or
deploy.branch) and pass that branch into any historical analytics/fetch code, or
else detect the absence of branch info on the deploy and reject deploy-scoped
historical queries; change logic in the getSiteDeploy branch where functions is
set (and mirror the same fix for the other occurrence around the 85-92 area) so
historical lookups use the deploy-derived branch instead of
siteInfo.build_settings?.repo_branch.
Comment on lines
+86
to
+101
| export function printHistoricalLogs(data: unknown, levelsToPrint: string[]): void { | ||
| const entries = Array.isArray(data) ? (data as { timestamp?: string; level?: string; message?: string }[]) : [] | ||
| const normalizedLevels = levelsToPrint.map((level) => level.toLowerCase()) | ||
|
|
||
| if (entries.length === 0) { | ||
| log('No logs found for the specified time range') | ||
| return | ||
| } | ||
|
|
||
| for (const entry of entries) { | ||
| const level = (entry.level ?? '').toLowerCase() | ||
| if (normalizedLevels.length > 0 && !normalizedLevels.includes(level)) { | ||
| continue | ||
| } | ||
| log(formatLogEntry(entry)) | ||
| } |
There was a problem hiding this comment.
Print a no-results message after applying the level filter.
If the API returns entries but all of them are filtered out by levelsToPrint, this exits silently. Historical mode is a bounded command, so an empty result for the selected filters should still say that nothing matched.
💡 Suggested change
export function printHistoricalLogs(data: unknown, levelsToPrint: string[]): void {
const entries = Array.isArray(data) ? (data as { timestamp?: string; level?: string; message?: string }[]) : []
const normalizedLevels = levelsToPrint.map((level) => level.toLowerCase())
+ let printed = 0
if (entries.length === 0) {
log('No logs found for the specified time range')
return
}
for (const entry of entries) {
const level = (entry.level ?? '').toLowerCase()
if (normalizedLevels.length > 0 && !normalizedLevels.includes(level)) {
continue
}
+ printed += 1
log(formatLogEntry(entry))
}
+
+ if (printed === 0) {
+ log('No logs found for the specified time range')
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/logs/log-api.ts` around lines 86 - 101, printHistoricalLogs
currently logs "No logs found..." only when the API returned zero entries, but
if entries exist and all are filtered out by levelsToPrint it exits silently;
change the logic in printHistoricalLogs to apply the level filter to entries
first (use normalizedLevels and entry.level) or track whether any entry was
printed (use a printedCount flag) and after the loop call log('No logs found for
the specified time range') when no entries matched; refer to symbols entries,
normalizedLevels, formatLogEntry, and log to locate the code to modify.
Add cleaner aliases so log commands are discoverable under their parent namespaces (deploy:logs, functions:logs, function:logs) while keeping the existing logs:* commands intact.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
docs/commands/logs.md (1)
102-102:⚠️ Potential issue | 🟡 MinorRegressed docs lint warnings in
logs:functionargument/flag text.Line 102 (
functionName) and Line 106 (“look up”) are the same lint issues flagged previously in this file and should be normalized again.✏️ Proposed doc-only fix
-- functionName - Name or ID of the function to stream logs for +- function-name - Name or ID of the function to stream logs for ... -- `deploy-id` (*string*) - Deploy ID to look up the function from +- `deploy-id` (*string*) - Deploy ID to find the function fromAlso applies to: 106-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/commands/logs.md` at line 102, Normalize the duplicated/incorrect argument/flag descriptions for the logs:function docs: update the "functionName" entry and the "look up" phrasing to match the canonical wording used elsewhere in this file (use the exact normalized phrase used previously for the function identifier description and the consistent "look-up" or "lookup" form), ensuring both occurrences (the symbol functionName and the phrase "look up") use the same lint-approved wording and punctuation as other command entries.src/commands/logs/edge-functions.ts (1)
19-19:⚠️ Potential issue | 🟠 Major
--deploy-idis silently ignored in historical mode.On Line 38, the historical path returns after a site-wide fetch;
deployIdfrom Line 19 is never applied there. This makes--deploy-id --from ...a no-op.🐛 Proposed safeguard (fail fast on unsupported combination)
if (options.from) { + if (deployId) { + log('`--deploy-id` is not supported with `--from`/`--to` for logs:edge-functions') + return + } + const fromMs = parseDateToMs(options.from as string) const toMs = options.to ? parseDateToMs(options.to as string) : Date.now() const url = buildEdgeFunctionLogsUrl({ siteId, from: fromMs, to: toMs })Also applies to: 38-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/logs/edge-functions.ts` at line 19, The deployId variable (deployId) is declared but ignored when the historical path returns after the site-wide fetch; update the handler in edge-functions.ts so that when historical/from mode is used you either apply deployId to the site-wide fetch call (pass deployId into the function that performs the site-wide fetch) or explicitly fail fast with a clear error if deploy-scoped historical queries are unsupported. Locate the branch that handles the historical path (the block that performs the site-wide fetch and returns) and: if the code can support deploy-scoped history, thread deployId into that fetch call (or into the helper like fetchSiteWideLogs/fetchLogs) so results are filtered by deploy; otherwise throw/console.error and exit when deployId is provided alongside the historical/from flags to prevent silent no-op behavior.src/commands/logs/log-api.ts (1)
91-97:⚠️ Potential issue | 🟡 MinorPrint feedback when filters exclude all historical entries.
Currently, users get no output when entries exist but none match
levelsToPrint.💡 Proposed UX fix
export function printHistoricalLogs(data: unknown, levelsToPrint: string[]): void { const entries = Array.isArray(data) ? (data as { timestamp?: string; level?: string; message?: string }[]) : [] const normalizedLevels = levelsToPrint.map((level) => level.toLowerCase()) + let printedCount = 0 if (entries.length === 0) { log('No logs found for the specified time range') return } for (const entry of entries) { const level = (entry.level ?? '').toLowerCase() if (normalizedLevels.length > 0 && !normalizedLevels.includes(level)) { continue } + printedCount += 1 log(formatLogEntry(entry)) } + + if (printedCount === 0) { + log('No logs found for the specified time range') + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/logs/log-api.ts` around lines 91 - 97, The loop over entries currently silently skips entries when none match normalizedLevels; add a boolean flag (e.g., printedAny) before the for-loop, set it true inside the loop when calling log(formatLogEntry(entry)), and after the loop if printedAny is false and entries.length > 0 and normalizedLevels.length > 0 call log(...) once with a helpful message like "No historical entries match the selected levels" so users get feedback when levelsToPrint filters out all historical entries; update references around entries, normalizedLevels, formatLogEntry and log accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/commands/functions.md`:
- Line 172: The docs text for the functionName argument is triggering lint
warnings; update the parameter description and the “look up” phrasing to avoid
the flagged wording: change the line that reads `functionName - Name or ID of
the function to stream logs for` to a clearer phrasing such as `functionName —
Name or ID of the function whose logs will be streamed`, and replace any
instance of the phrase “look up” (line with “look up”) with a more lint‑friendly
verb like “retrieve” or “find” to eliminate the warnings while preserving
meaning.
In `@src/commands/logs/edge-functions.ts`:
- Around line 88-94: The websocket message handler assumes every message is
valid JSON and will crash on malformed frames; wrap the JSON.parse and
subsequent processing in a try/catch inside the ws.on('message', ...) callback
(where you currently reference levelsToPrint, formatLogEntry, and log), ignore
or warn on parse errors and return early for invalid payloads, and only proceed
to check levelsToPrint and call log(formatLogEntry(...)) if parsing succeeds.
In `@src/commands/logs/log-api.ts`:
- Around line 60-62: formatLogEntry currently calls new
Date(entry.timestamp).toISOString() which will throw a RangeError for invalid
timestamp strings; modify formatLogEntry to validate the parsed Date before
calling toISOString (e.g., create const d = new Date(entry.timestamp) and check
isNaN(d.getTime()) or wrap in try/catch) and if invalid fall back to an empty
string or a safe default so that invalid entry.timestamp values no longer crash
the function.
---
Duplicate comments:
In `@docs/commands/logs.md`:
- Line 102: Normalize the duplicated/incorrect argument/flag descriptions for
the logs:function docs: update the "functionName" entry and the "look up"
phrasing to match the canonical wording used elsewhere in this file (use the
exact normalized phrase used previously for the function identifier description
and the consistent "look-up" or "lookup" form), ensuring both occurrences (the
symbol functionName and the phrase "look up") use the same lint-approved wording
and punctuation as other command entries.
In `@src/commands/logs/edge-functions.ts`:
- Line 19: The deployId variable (deployId) is declared but ignored when the
historical path returns after the site-wide fetch; update the handler in
edge-functions.ts so that when historical/from mode is used you either apply
deployId to the site-wide fetch call (pass deployId into the function that
performs the site-wide fetch) or explicitly fail fast with a clear error if
deploy-scoped historical queries are unsupported. Locate the branch that handles
the historical path (the block that performs the site-wide fetch and returns)
and: if the code can support deploy-scoped history, thread deployId into that
fetch call (or into the helper like fetchSiteWideLogs/fetchLogs) so results are
filtered by deploy; otherwise throw/console.error and exit when deployId is
provided alongside the historical/from flags to prevent silent no-op behavior.
In `@src/commands/logs/log-api.ts`:
- Around line 91-97: The loop over entries currently silently skips entries when
none match normalizedLevels; add a boolean flag (e.g., printedAny) before the
for-loop, set it true inside the loop when calling log(formatLogEntry(entry)),
and after the loop if printedAny is false and entries.length > 0 and
normalizedLevels.length > 0 call log(...) once with a helpful message like "No
historical entries match the selected levels" so users get feedback when
levelsToPrint filters out all historical entries; update references around
entries, normalizedLevels, formatLogEntry and log accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d567be6-f06e-403a-9f03-9f7e7928a0cf
📒 Files selected for processing (8)
docs/commands/deploy.mddocs/commands/functions.mddocs/commands/logs.mddocs/index.mdsrc/commands/deploy/deploy.tssrc/commands/logs/edge-functions.tssrc/commands/logs/index.tssrc/commands/logs/log-api.ts
✅ Files skipped from review due to trivial changes (2)
- docs/index.md
- src/commands/deploy/deploy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/logs/index.ts
|
|
||
| **Arguments** | ||
|
|
||
| - functionName - Name or ID of the function to stream logs for |
There was a problem hiding this comment.
Resolve docs lint warnings in the new argument/flag text.
Line 172 (functionName) and Line 176 (“look up”) will keep triggering lint-docs warnings; rewording here will keep docs clean.
✏️ Proposed doc-only fix
-- functionName - Name or ID of the function to stream logs for
+- function-name - Name or ID of the function to stream logs for
...
-- `deploy-id` (*string*) - Deploy ID to look up the function from
+- `deploy-id` (*string*) - Deploy ID to find the function fromAlso applies to: 176-176
🧰 Tools
🪛 GitHub Check: lint-docs
[warning] 172-172:
[vale] reported by reviewdog 🐶
[base.spelling] Spellcheck: did you really mean 'functionName'?
Raw Output:
{"message": "[base.spelling] Spellcheck: did you really mean 'functionName'?", "location": {"path": "docs/commands/functions.md", "range": {"start": {"line": 172, "column": 3}}}, "severity": "WARNING"}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/commands/functions.md` at line 172, The docs text for the functionName
argument is triggering lint warnings; update the parameter description and the
“look up” phrasing to avoid the flagged wording: change the line that reads
`functionName - Name or ID of the function to stream logs for` to a clearer
phrasing such as `functionName — Name or ID of the function whose logs will be
streamed`, and replace any instance of the phrase “look up” (line with “look
up”) with a more lint‑friendly verb like “retrieve” or “find” to eliminate the
warnings while preserving meaning.
Comment on lines
+88
to
+94
| ws.on('message', (data: string) => { | ||
| const logData = JSON.parse(data) as { level: string; message: string; timestamp?: string } | ||
| if (!levelsToPrint.includes(logData.level.toLowerCase())) { | ||
| return | ||
| } | ||
| log(formatLogEntry(logData)) | ||
| }) |
There was a problem hiding this comment.
Guard websocket parsing to prevent command crashes.
Line 89 assumes every frame is valid JSON; one malformed payload will throw and terminate the stream.
🛡️ Proposed defensive parsing
ws.on('message', (data: string) => {
- const logData = JSON.parse(data) as { level: string; message: string; timestamp?: string }
+ let logData: { level: string; message: string; timestamp?: string }
+ try {
+ logData = JSON.parse(data) as { level: string; message: string; timestamp?: string }
+ } catch {
+ log('Received malformed log payload')
+ return
+ }
+
if (!levelsToPrint.includes(logData.level.toLowerCase())) {
return
}
log(formatLogEntry(logData))
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/logs/edge-functions.ts` around lines 88 - 94, The websocket
message handler assumes every message is valid JSON and will crash on malformed
frames; wrap the JSON.parse and subsequent processing in a try/catch inside the
ws.on('message', ...) callback (where you currently reference levelsToPrint,
formatLogEntry, and log), ignore or warn on parse errors and return early for
invalid payloads, and only proceed to check levelsToPrint and call
log(formatLogEntry(...)) if parsing succeeds.
Comment on lines
+60
to
+62
| export function formatLogEntry(entry: { timestamp?: string; level?: string; message?: string }): string { | ||
| const timestamp = entry.timestamp ? new Date(entry.timestamp).toISOString() : '' | ||
| let levelString = entry.level ?? '' |
There was a problem hiding this comment.
Handle invalid timestamps safely in formatLogEntry.
Line 61 can throw RangeError when entry.timestamp is present but invalid, which can break both live and historical output flows.
🐛 Proposed resilient timestamp handling
export function formatLogEntry(entry: { timestamp?: string; level?: string; message?: string }): string {
- const timestamp = entry.timestamp ? new Date(entry.timestamp).toISOString() : ''
+ const parsedTimestamp = entry.timestamp ? Date.parse(entry.timestamp) : Number.NaN
+ const timestamp = Number.isNaN(parsedTimestamp) ? '' : new Date(parsedTimestamp).toISOString()
let levelString = entry.level ?? ''📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function formatLogEntry(entry: { timestamp?: string; level?: string; message?: string }): string { | |
| const timestamp = entry.timestamp ? new Date(entry.timestamp).toISOString() : '' | |
| let levelString = entry.level ?? '' | |
| export function formatLogEntry(entry: { timestamp?: string; level?: string; message?: string }): string { | |
| const parsedTimestamp = entry.timestamp ? Date.parse(entry.timestamp) : Number.NaN | |
| const timestamp = Number.isNaN(parsedTimestamp) ? '' : new Date(parsedTimestamp).toISOString() | |
| let levelString = entry.level ?? '' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/logs/log-api.ts` around lines 60 - 62, formatLogEntry currently
calls new Date(entry.timestamp).toISOString() which will throw a RangeError for
invalid timestamp strings; modify formatLogEntry to validate the parsed Date
before calling toISOString (e.g., create const d = new Date(entry.timestamp) and
check isNaN(d.getTime()) or wrap in try/catch) and if invalid fall back to an
empty string or a safe default so that invalid entry.timestamp values no longer
crash the function.
Contributor
|
Added deploy:logs and functions:logs (+ function:logs) as aliases so log commands are discoverable under their parent namespaces. This way agents and humans can find logs where they'd naturally look, under deploy and functions, instead of having to know about the logs:* namespace. The existing logs:deploy and logs:function commands are unchanged. Can revert this out if we don't want this. But I've seen agents trying to run |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
logs:edge-functionscommand for streaming and historical edge function logs--from/--todate options tologs:functionandlogs:edge-functionsfor historical log fetching via the analytics API--deploy-idoption to both commands to look up functions from a specific deploylogs:functionlog-api.tsutility for date parsing, REST API fetching, and log formattingNew files
src/commands/logs/log-api.ts— shared utilities for historical log fetchingsrc/commands/logs/edge-functions.ts— edge function log handlertests/integration/commands/logs/edge-functions.test.ts— tests forlogs:edge-functionsTest plan
npm run typecheck— no new type errorsnpm exec vitest -- run tests/integration/commands/logs/— all 15 tests passnpm exec vitest -- run tests/integration/commands/deploy/— 39/40 pass (1 pre-existing failure)./bin/run.js logs:function --helpshows--from,--to,--deploy-idoptions./bin/run.js logs:edge-functions --helpshows all options🤖 Generated with Claude Code