Skip to content
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

perf: add JSON.stringify to show load error message #42

Merged
merged 1 commit into from
Dec 15, 2023
Merged

Conversation

RGCHN
Copy link
Contributor

@RGCHN RGCHN commented Dec 15, 2023

Summary by CodeRabbit

  • Bug Fixes
    • Improved error messages across various asset loading functions to include the relevant URLs for easier troubleshooting.

Copy link
Contributor

coderabbitai bot commented Dec 15, 2023

Walkthrough

The recent updates across various modules enhance error and warning message details by incorporating JSON stringification of relevant data. This change makes debugging more straightforward by providing the exact URLs or data elements that are causing issues during asset loading operations.

Changes

File Path Change Summary
.../src/asset-manager.ts Enhanced error/warning messages to include JSON stringified data for URLs and bins in multiple methods.
.../src/downloader.ts Modified error messages in loadImage function to include JSON stringified representations of the source and error.

🐇✨
In the code where bugs would hide,
Now clearer messages abide.
With URLs in plain view,
We hop to fixes, swift and true.
🌟🐰

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 X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can reply to a review comment made by CodeRabbit.
  • You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
  • You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yaml

Commits Files that changed from the base of the PR and between bf1d08d and 1cc0dc1.
Files selected for processing (2)
  • packages/effects-core/src/asset-manager.ts (5 hunks)
  • packages/effects-core/src/downloader.ts (2 hunks)
Additional comments: 8
packages/effects-core/src/asset-manager.ts (5)
  • 170-170: The use of JSON.stringify to serialize the URL in the timeout rejection message is consistent with the PR's objective to improve error messaging.

  • 305-305: The error message for invalid bins source correctly uses JSON.stringify to serialize the bins object, which should help in debugging issues related to bins loading.

  • 328-328: The warning message for invalid font sources now includes the font URL serialized with JSON.stringify, aligning with the PR's goal to enhance error messages.

  • 486-486: The error message for failing to load JSON now includes the serialized URL, which is a good use of JSON.stringify to provide more detailed error information.

  • 497-497: The error message for failing to load bins is improved by including the serialized URL with JSON.stringify, which is in line with the PR's objectives.

packages/effects-core/src/downloader.ts (3)
  • 164-164: The use of JSON.stringify to serialize the source parameter in the error message is a good practice for debugging, as it provides a clear and complete representation of the data causing the issue.

  • 187-187: Wrapping the error object in JSON.stringify within the rejection callback will provide more detailed information about the error, which is beneficial for debugging purposes.

  • 161-167: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [164-187]

Ensure that the serialized error information does not contain sensitive data that could lead to security concerns or PII leakage.

@RGCHN RGCHN requested a review from yiiqii December 15, 2023 03:47
@yiiqii yiiqii merged commit 86e0bce into dev Dec 15, 2023
2 checks passed
@yiiqii yiiqii deleted the perf/log branch December 15, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants