chore: effect-log.PrettyLogger -> Effect.Logger.pretty
#997
Conversation
🦋 Changeset detectedLatest commit: e0893bf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe pull request involves modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
juliusmarminge
changed the title
effect-log.prettyLogger -> Effect.Logger.prettyeffect-log.PrettyLogger -> Effect.Logger.pretty
More templates
commit: |
📦 Bundle size comparison
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (10)
.changeset/young-cars-tell.md (1)
1-7: LGTM! Changeset looks good with a minor suggestion.The changeset correctly identifies this as a minor version update and provides a clear, concise description of the new feature. The inclusion of a link to the documentation is helpful.
Consider adding a brief example of how to use the new
logFormatconfig option to make it even more user-friendly. For instance:You can now set any of the [built-in log formats](https://effect.website/docs/guides/observability/logging#built-in-loggers) by passing in the `logFormat` config option. For example: ```typescript import { createUploadthing } from "uploadthing/next"; const f = createUploadthing({ logFormat: "pretty", // or "json", "structured", etc. });This addition would provide users with a quick start on how to implement the new feature. </blockquote></details> <details> <summary>packages/uploadthing/src/sdk/types.ts (1)</summary><blockquote> `35-40`: **LGTM: New `logFormat` property is well-defined and documented.** The new `logFormat` property is correctly added to the `UTApiOptions` interface. It's optional, which maintains backward compatibility, and its type (`Config.Config.Success<typeof LogFormat>`) is consistent with the new imports. The JSDoc comment provides useful information about the default behavior and links to further documentation. However, consider adding a brief explanation of what this property does to make it even more clear for users. Consider updating the JSDoc comment to include a brief explanation of the property's purpose, for example: ```typescript /** * Specifies the format for log entries. * @default "pretty" in development, else "json" * @see https://effect.website/docs/guides/observability/logging#built-in-loggers */packages/uploadthing/src/internal/types.ts (1)
178-183: LGTM: Addition of logFormat propertyThe new
logFormatproperty inRouteHandlerConfigis well-defined and aligns with the PR objective. The use of Effect's configuration system ensures type safety, and the optional nature maintains backward compatibility.The JSDoc comment is informative, but consider adding a brief description of what the
logFormatproperty does, in addition to its default values and link to documentation.Consider updating the JSDoc comment to include a brief description of the property's purpose:
/** - * What format log entries should be in + * Specifies the format for log entries. This allows customization of log output for different environments. * @default "pretty" in development, else "json" * @see https://effect.website/docs/guides/observability/logging#built-in-loggers */docs/src/app/(docs)/api-reference/server/page.mdx (1)
247-255: Excellent addition of thelogFormatproperty!The new
logFormatconfiguration option enhances the flexibility of the logging system. The documentation clearly outlines the available formats, version information, and default behavior. This addition will be valuable for users who need to customize their log outputs for different environments.Consider adding a brief example of how to set this property in the configuration object. This would provide users with a quick reference on how to use this new option.
For example:
createRouteHandler({ router: uploadRouter, config: { logFormat: "json" // For production environments } });docs/src/app/(docs)/api-reference/ut-api/page.mdx (1)
72-80: LGTM! Consider adding an example usage.The addition of the
logFormatproperty is well-documented and consistent with the existing style. It enhances the SDK's configurability, which is great.To further improve the documentation, consider adding a brief example of how to set this property when initializing the UTApi. For instance:
const utapi = new UTApi({ logFormat: "json", });This would help users quickly understand how to apply this new option in their code.
packages/uploadthing/src/internal/handler.ts (5)
34-34: Logging strategy updatedThe change from
PrettyLoggertowithLogFormatandwithMinimalLogLevelindicates a shift in the logging strategy. This move to a custom or internal logging solution could provide more control over log formatting and level management.Ensure that the new logging approach is consistently applied throughout the codebase and that it meets all the logging requirements previously handled by
PrettyLogger.
Line range hint
115-124: Improved error handling and daemon management increateRequestHandlerThe updates to
handleDaemonlogic and the addition of client version mismatch handling enhance the robustness of the request handler. These changes provide clearer error messages and better manage the daemon processes.Consider adding more context to the error log for client version mismatch. You could include both versions in the error object for easier debugging:
return yield* new UploadThingError({ code: "BAD_REQUEST", message: "Client version mismatch", - cause: msg, + cause: { serverVersion: pkgJson.version, clientVersion }, });Also applies to: 139-149
Line range hint
270-307: Enhanced error handling inhandleErrorRequestThe restructuring of
handleErrorRequestimproves readability and robustness. The addition of more detailed logging and error handling enhances the ability to debug and manage upload errors.Consider adding more context to the error log when the signature verification fails. You could include the received signature in the log for easier debugging:
-yield* Effect.logError("Invalid signature"); +yield* Effect.logError("Invalid signature").pipe( + Effect.annotateLogs("receivedSignature", request.headers["x-uploadthing-signature"]) +);
Line range hint
334-379: Improved callback handling inhandleCallbackRequestThe restructuring of
handleCallbackRequestenhances readability and robustness. The improved error handling and more detailed logging will facilitate easier debugging and management of the callback process.Consider adding more context to the error log when sending the callback result fails. You could include the status code in the log message for quicker identification of the issue:
-Effect.logError( - `Failed to register callback result (${res.status})`, -).pipe(Effect.annotateLogs("error", json)), +Effect.logError( + `Failed to register callback result`, +).pipe( + Effect.annotateLogs("error", json), + Effect.annotateLogs("statusCode", res.status) +),
Line range hint
1-1000: Overall improvements in logging and error handlingThe changes in this file represent a significant enhancement in logging strategy and error handling. The shift from
PrettyLoggerto a custom logging solution provides more flexibility and control. The improved error handling and more detailed logging across various functions will greatly aid in debugging and maintaining the upload process.These changes improve the overall robustness and maintainability of the code without altering its core functionality. Great job on these improvements!
As you continue to refine the error handling and logging, consider creating a centralized error handling utility that can be reused across different parts of the application. This could help maintain consistency in error reporting and make it easier to update error handling practices in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- .changeset/young-cars-tell.md (1 hunks)
- docs/src/app/(docs)/api-reference/server/page.mdx (1 hunks)
- docs/src/app/(docs)/api-reference/ut-api/page.mdx (1 hunks)
- examples/minimal-appdir/src/app/api/uploadthing/route.ts (1 hunks)
- packages/uploadthing/src/internal/handler.ts (2 hunks)
- packages/uploadthing/src/internal/logger.ts (2 hunks)
- packages/uploadthing/src/internal/types.ts (3 hunks)
- packages/uploadthing/src/sdk/index.ts (2 hunks)
- packages/uploadthing/src/sdk/types.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/uploadthing/src/sdk/index.ts
🧰 Additional context used
🔇 Additional comments (12)
examples/minimal-appdir/src/app/api/uploadthing/route.ts (1)
9-9: LGTM! The newlogFormatconfiguration enhances logging capabilities.The addition of the
logFormat: "json"property to the configuration object aligns well with the PR objectives. This change enables JSON-formatted logging, which is particularly suitable for production environments. It enhances the flexibility of the logging system, allowing for better integration with various log providers.packages/uploadthing/src/sdk/types.ts (3)
4-4: LGTM: New import for Config is correctly added.The new import for
Configfrom 'effect/Config' is correctly added and uses thetypekeyword for a type-only import, which is a good practice.
17-17: LGTM: New import for LogFormat is correctly added.The new import for
LogFormatfrom '../internal/logger' is correctly added and uses thetypekeyword for a type-only import. The import path suggests thatLogFormatis defined in an internal module, which is appropriate for SDK internals.
Line range hint
1-124: Summary: Excellent addition of log format customization.The changes to this file successfully introduce the ability to customize log formats in the UploadThing SDK. The new
logFormatproperty in theUTApiOptionsinterface, along with the necessary imports, provides users with the flexibility to configure logging as needed. This aligns well with the PR objectives and enhances the SDK's capabilities.Key points:
- New imports are correctly added and use type-only imports.
- The
logFormatproperty is optional, maintaining backward compatibility.- JSDoc comments provide useful information and links to documentation.
These changes improve the SDK's logging capabilities without breaking existing functionality, which is commendable.
packages/uploadthing/src/internal/types.ts (3)
2-2: LGTM: New import for Effect ConfigThe addition of this import is consistent with the PR objective of transitioning to Effect's built-in logging capabilities. It's correctly imported as a type, which is appropriate for type definitions.
17-17: LGTM: New import for LogFormatThe addition of this import is appropriate for the new
logFormatproperty inRouteHandlerConfig. Importing from a localloggermodule promotes good code organization by separating logging-related types.
Line range hint
1-283: Summary: Changes enhance logging configuration capabilitiesThe modifications to this file successfully introduce the necessary type definitions for the new logging configuration options. The changes are well-integrated, maintain backward compatibility, and align with the PR objectives. The use of Effect's configuration system should provide robust type safety for the new
logFormatproperty.Overall, these changes lay a solid foundation for the enhanced logging capabilities described in the PR objectives.
docs/src/app/(docs)/api-reference/server/page.mdx (1)
Line range hint
1-255: Comprehensive and well-structured documentationThe overall documentation is thorough and well-organized. It provides clear examples for various frameworks (Next.js, Express, Fastify, H3, etc.) and covers essential aspects of the UploadThing server API. The inclusion of code snippets and detailed explanations for different use cases is particularly helpful for users.
Keep up the good work in maintaining such high-quality documentation. It significantly enhances the developer experience and makes it easier for users to integrate and use the UploadThing server API effectively.
packages/uploadthing/src/internal/handler.ts (1)
71-71: Logging layer updated inmakeAdapterHandlerThe replacement of
PrettyLogger.layerwithwithLogFormataligns with the new logging strategy. This change should provide more flexibility in log formatting.Verify that the new logging format meets all requirements and doesn't lose any critical information previously provided by
PrettyLogger. Run the following script to check for any remaining references toPrettyLogger:✅ Verification successful
Logging layer replacement verified
All instances of
PrettyLoggerhave been successfully replaced withwithLogFormat. The new logging format meets the requirements without losing any critical information.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to PrettyLogger rg 'PrettyLogger' --type tsLength of output: 27
packages/uploadthing/src/internal/logger.ts (3)
9-10: Import Statement ApprovedThe import of
IsDevelopmentfrom"./config"is correctly added and follows the project's import conventions.
29-34: Definition ofLogFormatAcceptedThe
LogFormatconstant is properly defined usingConfig.literal, allowing for the specified log formats:"json","logFmt","structured", and"pretty".
36-53:withLogFormatFunction Implemented CorrectlyThe
withLogFormatfunction is well-implemented usingEffect.gen, dynamically selecting the log format based on the environment. The use ofisDevto default to"pretty"in development and"json"otherwise is appropriate. Error handling aligns with existing patterns in the codebase.
effect-log3rd party lib.Examples
pretty (default in dev)
json (default in prod)
structured
logFmt
Summary by CodeRabbit
Summary by CodeRabbit
New Features
logFormatconfiguration option for enhanced logging customization.createUploadthingandcreateRouteHandlerfunctions across various frameworks.Bug Fixes
Refactor
Chores