Skip to content

Comments

Allow null throwable when logging errors#28

Merged
VelikovPetar merged 1 commit intomainfrom
error-log-with-null-throwable
Mar 31, 2025
Merged

Allow null throwable when logging errors#28
VelikovPetar merged 1 commit intomainfrom
error-log-with-null-throwable

Conversation

@JcMinarro
Copy link
Contributor

@JcMinarro JcMinarro commented Mar 27, 2025

🎯 Goal

There are some cases where we want to log an error but the exception/throwable can be null, in that case, an if-else check needs to be done before logging this error. The StreamLog already handles this case internally.

Imagine the following class:

data class Error(
  val message: String,
  val throwable: Throwable? = null
)

With previous implementation, we would have something like:

fun handleError(error: Error) {
  if (error.throwable != null) {
    logger.e(error.throwable) { error.message }
  } else {
    logger.e { error.message }
  }
  [...]
}

That can be changed by:

fun handleError(error: Error) {
  logger.e(error.throwable) { error.message }
  [...]
}

@JcMinarro JcMinarro requested a review from a team March 27, 2025 07:24
@JcMinarro JcMinarro force-pushed the error-log-with-null-throwable branch from 2c9f26b to 78a5557 Compare March 27, 2025 07:26
VelikovPetar
VelikovPetar approved these changes Mar 28, 2025
View reviewed changes
Copy link

@VelikovPetar VelikovPetar left a comment

Choose a reason for hiding this comment

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

Looks good!
We should just make sure that the new version would be compatible with the older ones. Ex:

  • integrator uses the newest (unreleased) version
  • Chat/Video are on a different older version of log

@JcMinarro
Copy link
Contributor Author

JcMinarro commented Mar 28, 2025

Looks good! We should just make sure that the new version would be compatible with the older ones. Ex:

  • integrator uses the newest (unreleased) version
  • Chat/Video are on a different older version of log

I think it should because previous methods are maintained and even we reduce restrictions (nullable values is less restricted than non-nullable values)

@VelikovPetar
Copy link

VelikovPetar commented Mar 31, 2025

Looks good! We should just make sure that the new version would be compatible with the older ones. Ex:

  • integrator uses the newest (unreleased) version
  • Chat/Video are on a different older version of log

I think it should because previous methods are maintained and even we reduce restrictions (nullable values is less restricted than non-nullable values)

Let's just test this scenario before updating the Chat/Video SDKs to use the newer version!

@VelikovPetar VelikovPetar merged commit 36f7254 into main Mar 31, 2025
4 checks passed
@VelikovPetar VelikovPetar deleted the error-log-with-null-throwable branch March 31, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andremion andremion andremion approved these changes

@VelikovPetar VelikovPetar VelikovPetar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JcMinarro @VelikovPetar @andremion