fix: eliminate 'socket hang up' uncaught exceptions at the request source - #31
Open
sean-roberts wants to merge 1 commit into
Open
fix: eliminate 'socket hang up' uncaught exceptions at the request source#31sean-roberts wants to merge 1 commit into
sean-roberts wants to merge 1 commit into
Conversation
…urce The process-guards fix (PR #29) catches the promise-based resets (swallowed as warns) but the persisting 'Invoke Error socket hang up' errors are a different path: an uncaughtException from a socket 'error' on a legacy-http request (node:_http_client) that some dependency never handles. The Lambda/Netlify runtime logs that itself, independent of our process-level uncaughtException handler — so the handler can't suppress it. Fix at the source instead: http-guard.ts patches http/https .request/.get to attach a default 'error' listener to every outbound request, so a socket reset is handled and never becomes uncaught. Self-installs on import; imported FIRST in mcp.ts and oauth-server.ts so it patches before any dependency captures a reference. Node-only, never edge. Also drop the process-guard swallow logs from warn to debug — they were adding ~17k/2d of unactionable warns now that the http-guard handles most at the request. 71/71 tests pass; typecheck clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ Deploy Preview for ntl-mcp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for netlify-mcp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for mcp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request adds an import-time guard for outbound Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to This localized change adds request-level handling for socket errors and reduces unactionable guard logging; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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
1 participant
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.
Problem
After PR #29,
socket hang uperrors persist on the dashboard (~2k/hr). Diagnosis from Datadog:transient network rejection swallowedin 2 days.Invoke Error socket hang up— an uncaughtException from a socket'error'on a legacy-httprequest (node:_http_client). The Lambda/Netlify runtime logs that itself, independent of our processuncaughtExceptionhandler, so the handler can't suppress it (the platform-override fragility we flagged).Fix
http-guard.tspatcheshttp/https.request/.getto attach a default'error'listener to every outbound request → a socket reset is handled and never becomes uncaught. Self-installs on import; imported first inmcp.ts/oauth-server.tsso it's in place before any dependency captures a reference. Node-only.Also: dropped the process-guard swallow logs
warn→debug(they were ~17k/2d of unactionable noise now that the guard handles most at the request).Verify
Invoke Error socket hang upcount drops.Note
Some of these logs have a blank
function_name; if any originate in the platform's own outbound calls (not our deps), the guard won't reach those — those would need a dashboard exclusion / a note to Netlify.🤖 Generated with Claude Code