Skip to content

LoggerStore + NetworkLogger + Redacted + Patterns #328

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

wow-such-amazing
Copy link

@wow-such-amazing wow-such-amazing commented Apr 2, 2025

I've extracted fields that are responsible for redacting into its own struct Redacted from NetworkLogger.Configuration. I then created an internal type Redacted.Patterns that mimics what was inside NetworkLogger itself and what was inside NetworkLogger+Redacting.

This way we can set separately Redacted struct onto the NetworkLogger and LoggerStore configurations and they would follow the same approach.

We can also set same Redacted for both if we'd like to follow the same logic in both.

Let me know what you think 🙏

Addresses #327

P.S. Sorry for spamming with many comments here 😅

@wow-such-amazing
Copy link
Author

I probably need to make some changes to the demo project, let me check 👀

@wow-such-amazing
Copy link
Author

Alright, demo project compiles. But still need to update docs. And also been looking at the implementation and maybe the best route is actually to extract that StoreRequest logic from LoggerStore and create a new small type like RequestLogger 🤔

@wow-such-amazing
Copy link
Author

wow-such-amazing commented Apr 2, 2025

Also because configuration property of LoggerStore is a variable that could be modified on the go. Which means that if we modify redacted then we also need to update patterns. So either we need to address this inside LoggerStore or a better way is probably a new type 🤔

@wow-such-amazing
Copy link
Author

Hm, I also realized this is a breaking change now 😅

@wow-such-amazing
Copy link
Author

I have pushed a commit with a dedicated RequestsLogger. Let me know what you think about all that playing around 😄

@wow-such-amazing
Copy link
Author

Just tried it in my app using the fork branch and it works 🕺

    public func setup() {
        UserSettings.shared.listDisplayOptions.content.showTaskDescription = true

        var redacted = Pulse.Redacted()
        redacted.sensitiveHeaders = ["Authorization"]

        NetworkLogger.shared = NetworkLogger(store: self.store) {
            $0.redacted = redacted
        }

        RequestsLogger.shared = RequestsLogger(store: self.store) {
            $0.redacted = redacted
        }
    }
Screenshot 2025-04-02 at 14 47 30

@kean
Copy link
Owner

kean commented Apr 11, 2025

Hey, thanks for the PR, @wow-such-amazing. I'll review it as soon as I get a chance.

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.

2 participants