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

fix: use separate logger for ui/sw #217

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Apr 23, 2024

  • chore: use prefixLogger
  • chore: use new logger in sw.ts
  • fix: use separate logger for ui/sw

Title

fix: use separate logger for ui/sw

Description

Fixes #139

All logs now:

  1. include the host.
    • For UI: useful for redirect and config page where things are logged from iframe
    • For SW: useful for debugging different messages from service worker loaded on page in iframe vs main page.
  2. have a global prefix: helia:service-worker-gateway:
  3. have a distinct prefix.
    • For UI: helia:service-worker-gateway:ui
    • For SW: helia:service-worker-gateway:sw
  4. are injected into components that need them

Notes & open questions

On redirect page

filtering console logs by helia:service-worker-gateway:sw

image

filtering console logs by helia:service-worker-gateway:ui

image

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@SgtPooki SgtPooki linked an issue Apr 23, 2024 that may be closed by this pull request
Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

self review. updates incoming.

export const log = logObj
export const error = logObj.error
export const trace = logObj.trace
const swLogPrefix = `helia:service-worker-gateway:sw:${host}`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const swLogPrefix = `helia:service-worker-gateway:sw:${host}`
const swLogPrefix = `helia:sw-gateway:sw:${host}`

Any thoughts on making the logs a little shorter since we use sw as an acception abbreviation elsewhere too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeaaa i thought about that. Theyre pretty long, so if you think shorter is fine im absolutely a fan.

I think uniqueness is the most important here, instead of verbosity. If someone is looking at these logs, theyre likely aware its a "service-worker-gateway"

Copy link
Member Author

Choose a reason for hiding this comment

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

Is "helia:gateway:sw" & "helia:gateway:ui" too obtuse?

Copy link
Member

@2color 2color left a comment

Choose a reason for hiding this comment

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

Really nice!

@SgtPooki SgtPooki merged commit a96837b into main Apr 24, 2024
20 checks passed
@SgtPooki SgtPooki deleted the 139-use-separate-logger-in-sw-and-frontend branch April 24, 2024 18:23
@lidel lidel mentioned this pull request Apr 23, 2024
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.

Use separate logger in sw and frontend
2 participants