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

Allow overridden parameter types on GET/DELETE #264

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jasperblues
Copy link

I'm not sure if this will be useful to others (hopefully, as it will alleviate our need to sync with upstream) however I needed to add support for DELETE with a JSON request body. (API didn't strictly adhere to spec).

For uniformity, provided also for GET. (In the past I have seen GET reqs with url encoded params along with a serialized body - this would not be supported).

@jasperblues
Copy link
Author

jasperblues commented Sep 28, 2021

@3lvis After the above PR, have another, which is the introduction of an ErrorLogger:

import Foundation

public protocol ErrorLogger {

    func log(_ message: String)

    func flush()

}

class ConsoleErrorLogger : ErrorLogger {

    func log(_ message: String) {
        print(message)
    }

    func flush() {} // Not required for console 

}

Default (provided) implementation logs to console, just as currently, custom version can, for example, log to Sentry.

@mkll
Copy link

mkll commented Sep 29, 2021

@jasperblues It seems to me that the name chosen is not entirely successful. We can log not only errors, but any messages in general, right?

Maybe a name like Logger would be more appropriate?

@mkll
Copy link

mkll commented Sep 29, 2021

After all, the name ErrorHandler implies an error handler, not simple message logging. I mean error trapping, recovery, etc.

@jasperblues
Copy link
Author

Ah yes @mkll you're right, that was a terrible choice! ErrorLogger would be better. (This only intercepts the logging that occurs on error, rest of the console logging is left alone).

@jasperblues
Copy link
Author

(More complicated, but better) A LogProvider that emits a (non-shared) instance of ErrorLogger will to prevent two errors being logged at the same time from entangling.

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