Skip to content

Conversation

moreorover
Copy link

I had a quick look at the source, I think this should solve the issue, however, testing is required.

@louisameline
Copy link
Collaborator

I haven't tested, but from what I read, the function previously returned a string in case of error if logging is enabled, and now would throw an exception. Am I right?
It's cleaner of course but this would be a breaking change for the people who leave logging enabled in production. Maybe you could change your PR to throw only if logging is disabled?
I know it sounds stupid and returning strings is not the way to go, but the lib is going to be rewritten soon with proper error handling everywhere, so we might not want to break backward compatibility by trying to fix things now.

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