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

Update MockWebServer.kt to fix runtime crash caused by "okhttp3.mockwebserver.MockWebServer" exceeds limit of 23 characters. #6225

Conversation

@yschimke
Copy link
Collaborator

I would prefer making this change in the same place as the other runtime long class names.

private fun loggerTag(loggerName: String): String {

The comments indicate I hit this before in testing, and then just didn't fix correctly.

@yschimke
Copy link
Collaborator

@russell-shizhen Just checking, do you want me to make the changes discussed above or should I leave it with you?

@russell-shizhen
Copy link
Contributor Author

@yschimke ,

According to the implementation in side

private fun loggerTag(loggerName: String): String {

private fun loggerTag(loggerName: String): String {
    // We need to handle long logger names before they hit Log.
    // java.lang.IllegalArgumentException: Log tag "okhttp3.mockwebserver.MockWebServer" exceeds limit of 23 characters
    return knownLoggers[loggerName] ?: loggerName.take(23)
  }

loggerName.take(23) will take a log tag "okhttp3.mockwebserver." (23 chars) which may not be that helpful and sometimes misleading due to its inaccuracy. Maybe some tag string that is self-explained will be more helpful for debugging.

@russell-shizhen
Copy link
Contributor Author

@yschimke , I am not sure about "should I leave it with you", what can I do?

@yschimke
Copy link
Collaborator

Well either

a) You intend to make further changes and we can work through the PR
b) I can submit an alternate PR

If you look at the method I sent, it already does what you are suggesting, using known abbreviations when appropriate. So you could add a specific mapping for MockWebServer also (use a string instead of a class reference).

this[OkHttpClient::class.java.name] = "okhttp.OkHttpClient"

@russell-shizhen
Copy link
Contributor Author

@yschimke , Thank you for your advices. I have made the PR #6229

@yschimke yschimke closed this Aug 22, 2020
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.

None yet

2 participants