fix: filter out more user error messages#8230
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change expands the user input error filter in the error reporting handler by adding four new message substrings to the Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
📊 Benchmark resultsComparing with c3ffebf
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
site/netlify/functions/error-reporting.ts (1)
15-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize error-message matching to avoid case-sensitive misses.
At Line 15 and Line 21, matching is case-sensitive, so equivalent messages with different capitalization won’t be filtered.
Proposed fix
-const isUserInputError = (message: unknown): boolean => - typeof message === 'string' && USER_INPUT_ERROR_MESSAGE_PATTERNS.some((pattern) => message.includes(pattern)) +const isUserInputError = (message: unknown): boolean => { + if (typeof message !== 'string') { + return false + } + + const normalizedMessage = message.toLowerCase() + return USER_INPUT_ERROR_MESSAGE_PATTERNS.some((pattern) => + normalizedMessage.includes(pattern.toLowerCase()), + ) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/netlify/functions/error-reporting.ts` around lines 15 - 21, The isUserInputError function and USER_INPUT_ERROR_MESSAGE_PATTERNS perform case-sensitive string matching, so normalize case before comparing: convert the incoming message to a lowercase string (e.g. messageLower = String(message).toLowerCase()) and compare against lowercased pattern values (pattern.toLowerCase()) when using includes; update isUserInputError to use messageLower.includes(pattern.toLowerCase()) so variants in capitalization are correctly detected (references: isUserInputError and USER_INPUT_ERROR_MESSAGE_PATTERNS).
🧹 Nitpick comments (1)
site/netlify/functions/error-reporting.ts (1)
14-17: ⚡ Quick winAdd regression tests for each newly filtered message pattern.
These string filters are easy to break with message wording drift; tests will lock expected behavior for all four additions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/netlify/functions/error-reporting.ts` around lines 14 - 17, The new filtered message strings ('Not authorized to view the currently linked project', 'could not retrieve project', "You don't appear to be in a folder that is linked to a project", 'EADDRINUSE: address already in use') need regression tests to prevent accidental breaks: add unit tests that assert the error-filtering function (the filter/listing logic in error-reporting.ts — e.g., the array of filtered messages and the function that checks messages, locate symbols like the filtered messages array and the function that evaluates error text) returns true (is filtered) for each exact message and false for slight variants; place tests alongside existing error-reporting tests, covering all four strings as independent test cases to lock expected behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@site/netlify/functions/error-reporting.ts`:
- Around line 15-21: The isUserInputError function and
USER_INPUT_ERROR_MESSAGE_PATTERNS perform case-sensitive string matching, so
normalize case before comparing: convert the incoming message to a lowercase
string (e.g. messageLower = String(message).toLowerCase()) and compare against
lowercased pattern values (pattern.toLowerCase()) when using includes; update
isUserInputError to use messageLower.includes(pattern.toLowerCase()) so variants
in capitalization are correctly detected (references: isUserInputError and
USER_INPUT_ERROR_MESSAGE_PATTERNS).
---
Nitpick comments:
In `@site/netlify/functions/error-reporting.ts`:
- Around line 14-17: The new filtered message strings ('Not authorized to view
the currently linked project', 'could not retrieve project', "You don't appear
to be in a folder that is linked to a project", 'EADDRINUSE: address already in
use') need regression tests to prevent accidental breaks: add unit tests that
assert the error-filtering function (the filter/listing logic in
error-reporting.ts — e.g., the array of filtered messages and the function that
checks messages, locate symbols like the filtered messages array and the
function that evaluates error text) returns true (is filtered) for each exact
message and false for slight variants; place tests alongside existing
error-reporting tests, covering all four strings as independent test cases to
lock expected behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26f210e3-6163-4d6a-beaf-bdfa0ca362a2
📒 Files selected for processing (1)
site/netlify/functions/error-reporting.ts
🎉 Thanks for submitting a pull request! 🎉
Summary
This filters out a few extra user errors that I think we do not need to have in our monitoring.
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)