-
Notifications
You must be signed in to change notification settings - Fork 5
Feature/http logger #272
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.
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
base: main
Are you sure you want to change the base?
Feature/http logger #272
Conversation
|
📝 Documentation updates detected! You can review documentation updates here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mrge found 8 issues across 4 files. View them in mrge.io
|
|
||
| | Parameter | Type | Description | | ||
| |-----------|------|-------------| | ||
| | `logData.error` | boolean | Whether the request resulted in an error | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error parameter is marked as required in the README table but is optional in the code
| | `logData.method` | string | HTTP method used (GET, POST, etc.) | | ||
| | `logData.url` | string | Request URL | | ||
| | `logData.startTime` | Date | When the request started | | ||
| | `logData.headers` | Object | Response headers | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The headers parameter is marked as required in the README table but is optional in the code
| | `logData.startTime` | Date | When the request started | | ||
| | `logData.headers` | Object | Response headers | | ||
| | `logData.statusCode` | number | Response status code | | ||
| | `logData.requestSize` | number | Size of request in bytes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The requestSize parameter is marked as required in the README table but is optional in the code
| | `logData.headers` | Object | Response headers | | ||
| | `logData.statusCode` | number | Response status code | | ||
| | `logData.requestSize` | number | Size of request in bytes | | ||
| | `logData.responseSize` | number | Size of response in bytes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The responseSize parameter is marked as required in the README table but is optional in the code
| | `logData.statusCode` | number | Response status code | | ||
| | `logData.requestSize` | number | Size of request in bytes | | ||
| | `logData.responseSize` | number | Size of response in bytes | | ||
| | `logData.isStreaming` | boolean | Whether this is a streaming response | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isStreaming parameter is marked as required in the README table but is optional in the code
| const chunkSize = Buffer.isBuffer(chunk) ? chunk.length : Buffer.byteLength(chunk, encoding || 'utf8') | ||
| requestSize += chunkSize | ||
| } | ||
| return originalWrite.apply(this, arguments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using arguments object directly is discouraged in modern JavaScript. Consider using rest parameters
| } | ||
| } | ||
|
|
||
| // eslint-disable-next-line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eslint-disable comments should specify which rule is being disabled
| } | ||
|
|
||
| // eslint-disable-next-line | ||
| requestLogger(require("http")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using require() in an ES module can cause compatibility issues
Summary by mrge
Added a new @flatfile/http-logger package to log HTTP and fetch requests in Node.js, including request/response sizes, status codes, and streaming detection.