Simplify error responses (#3762)

This commit is contained in:
David Perez 2024-08-16 10:07:56 -05:00 committed by GitHub
parent 3bed2581af
commit 48817f0fe4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 33 additions and 80 deletions

View File

@ -46,7 +46,6 @@ sealed class RegisterResponseJson {
/** /**
* Represents the json body of an invalid register request. * Represents the json body of an invalid register request.
* *
* @param message
* @param validationErrors a map where each value is a list of error messages for each key. * @param validationErrors a map where each value is a list of error messages for each key.
* The values in the array should be used for display to the user, since the keys tend to come * The values in the array should be used for display to the user, since the keys tend to come
* back as nonsense. (eg: empty string key) * back as nonsense. (eg: empty string key)
@ -54,18 +53,17 @@ sealed class RegisterResponseJson {
@Serializable @Serializable
data class Invalid( data class Invalid(
@SerialName("message") @SerialName("message")
val message: String?, private val invalidMessage: String? = null,
@SerialName("Message")
private val errorMessage: String? = null,
@SerialName("validationErrors") @SerialName("validationErrors")
val validationErrors: Map<String, List<String>>?, val validationErrors: Map<String, List<String>>?,
) : RegisterResponseJson() ) : RegisterResponseJson() {
/**
/** * A generic error message.
* A different register error with a message. */
*/ val message: String? get() = invalidMessage ?: errorMessage
@Serializable }
data class Error(
@SerialName("Message")
val message: String?,
) : RegisterResponseJson()
} }

View File

@ -22,7 +22,6 @@ sealed class SendVerificationEmailResponseJson {
/** /**
* Represents the json body of an invalid request. * Represents the json body of an invalid request.
* *
* @param message
* @param validationErrors a map where each value is a list of error messages for each key. * @param validationErrors a map where each value is a list of error messages for each key.
* The values in the array should be used for display to the user, since the keys tend to come * The values in the array should be used for display to the user, since the keys tend to come
* back as nonsense. (eg: empty string key) * back as nonsense. (eg: empty string key)
@ -30,18 +29,17 @@ sealed class SendVerificationEmailResponseJson {
@Serializable @Serializable
data class Invalid( data class Invalid(
@SerialName("message") @SerialName("message")
val message: String?, private val invalidMessage: String? = null,
@SerialName("Message")
private val errorMessage: String? = null,
@SerialName("validationErrors") @SerialName("validationErrors")
val validationErrors: Map<String, List<String>>?, val validationErrors: Map<String, List<String>>?,
) : SendVerificationEmailResponseJson() ) : SendVerificationEmailResponseJson() {
/**
/** * A generic error message.
* A different error with a message. */
*/ val message: String? get() = invalidMessage ?: errorMessage
@Serializable }
data class Error(
@SerialName("Message")
val message: String?,
) : SendVerificationEmailResponseJson()
} }

View File

@ -43,10 +43,6 @@ class IdentityServiceImpl(
codes = listOf(400, 429), codes = listOf(400, 429),
json = json, json = json,
) )
?: bitwardenError.parseErrorBodyOrNull<RegisterResponseJson.Error>(
code = 429,
json = json,
)
?: throw throwable ?: throw throwable
} }
@ -121,10 +117,6 @@ class IdentityServiceImpl(
codes = listOf(400, 429), codes = listOf(400, 429),
json = json, json = json,
) )
?: bitwardenError.parseErrorBodyOrNull<RegisterResponseJson.Error>(
code = 429,
json = json,
)
?: throw throwable ?: throw throwable
} }

View File

@ -816,10 +816,6 @@ class AuthRepositoryImpl(
?: it.message, ?: it.message,
) )
} }
is RegisterResponseJson.Error -> {
RegisterResult.Error(it.message)
}
} }
}, },
onFailure = { RegisterResult.Error(errorMessage = null) }, onFailure = { RegisterResult.Error(errorMessage = null) },

View File

@ -22,7 +22,6 @@ import com.x8bit.bitwarden.data.platform.util.asSuccess
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonNull import kotlinx.serialization.json.JsonNull
import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonObject
import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.JsonPrimitive
@ -41,9 +40,7 @@ class IdentityServiceTest : BaseServiceTest() {
private val identityService = IdentityServiceImpl( private val identityService = IdentityServiceImpl(
unauthenticatedIdentityApi = unauthenticatedIdentityApi, unauthenticatedIdentityApi = unauthenticatedIdentityApi,
json = Json { json = json,
ignoreUnknownKeys = true
},
deviceModelProvider = deviceModelProvider, deviceModelProvider = deviceModelProvider,
) )
@ -154,7 +151,7 @@ class IdentityServiceTest : BaseServiceTest() {
val result = identityService.register(registerRequestBody) val result = identityService.register(registerRequestBody)
assertEquals( assertEquals(
RegisterResponseJson.Invalid( RegisterResponseJson.Invalid(
message = "The model state is invalid.", invalidMessage = "The model state is invalid.",
validationErrors = mapOf("" to listOf("Email '' is already taken.")), validationErrors = mapOf("" to listOf("Email '' is already taken.")),
), ),
result.getOrThrow(), result.getOrThrow(),
@ -167,8 +164,9 @@ class IdentityServiceTest : BaseServiceTest() {
server.enqueue(response) server.enqueue(response)
val result = identityService.register(registerRequestBody) val result = identityService.register(registerRequestBody)
assertEquals( assertEquals(
RegisterResponseJson.Error( RegisterResponseJson.Invalid(
message = "Slow down! Too many requests. Try again soon.", errorMessage = "Slow down! Too many requests. Try again soon.",
validationErrors = null,
), ),
result.getOrThrow(), result.getOrThrow(),
) )
@ -328,7 +326,7 @@ class IdentityServiceTest : BaseServiceTest() {
val result = identityService.registerFinish(registerFinishRequestBody) val result = identityService.registerFinish(registerFinishRequestBody)
assertEquals( assertEquals(
RegisterResponseJson.Invalid( RegisterResponseJson.Invalid(
message = "The model state is invalid.", invalidMessage = "The model state is invalid.",
validationErrors = mapOf("" to listOf("Email '' is already taken.")), validationErrors = mapOf("" to listOf("Email '' is already taken.")),
), ),
result.getOrThrow(), result.getOrThrow(),
@ -341,8 +339,9 @@ class IdentityServiceTest : BaseServiceTest() {
server.enqueue(response) server.enqueue(response)
val result = identityService.registerFinish(registerFinishRequestBody) val result = identityService.registerFinish(registerFinishRequestBody)
assertEquals( assertEquals(
RegisterResponseJson.Error( RegisterResponseJson.Invalid(
message = "Slow down! Too many requests. Try again soon.", errorMessage = "Slow down! Too many requests. Try again soon.",
validationErrors = null,
), ),
result.getOrThrow(), result.getOrThrow(),
) )

View File

@ -3712,7 +3712,9 @@ class AuthRepositoryTest {
kdfIterations = DEFAULT_KDF_ITERATIONS.toUInt(), kdfIterations = DEFAULT_KDF_ITERATIONS.toUInt(),
), ),
) )
} returns RegisterResponseJson.Invalid("message", mapOf()).asSuccess() } returns RegisterResponseJson
.Invalid(invalidMessage = "message", validationErrors = mapOf())
.asSuccess()
val result = repository.register( val result = repository.register(
email = EMAIL, email = EMAIL,
@ -3746,7 +3748,7 @@ class AuthRepositoryTest {
) )
} returns RegisterResponseJson } returns RegisterResponseJson
.Invalid( .Invalid(
message = "message", invalidMessage = "message",
validationErrors = mapOf("" to listOf("expected")), validationErrors = mapOf("" to listOf("expected")),
) )
.asSuccess() .asSuccess()
@ -3762,38 +3764,6 @@ class AuthRepositoryTest {
assertEquals(RegisterResult.Error(errorMessage = "expected"), result) assertEquals(RegisterResult.Error(errorMessage = "expected"), result)
} }
@Test
fun `register returns Error body should return Error with message`() = runTest {
coEvery { identityService.preLogin(EMAIL) } returns PRE_LOGIN_SUCCESS.asSuccess()
coEvery {
identityService.register(
body = RegisterRequestJson(
email = EMAIL,
masterPasswordHash = PASSWORD_HASH,
masterPasswordHint = null,
captchaResponse = null,
key = ENCRYPTED_USER_KEY,
keys = RegisterRequestJson.Keys(
publicKey = PUBLIC_KEY,
encryptedPrivateKey = PRIVATE_KEY,
),
kdfType = KdfTypeJson.PBKDF2_SHA256,
kdfIterations = DEFAULT_KDF_ITERATIONS.toUInt(),
),
)
} returns RegisterResponseJson.Error(message = "message").asSuccess()
val result = repository.register(
email = EMAIL,
masterPassword = PASSWORD,
masterPasswordHint = null,
captchaToken = null,
shouldCheckDataBreaches = false,
isMasterPasswordStrong = true,
)
assertEquals(RegisterResult.Error(errorMessage = "message"), result)
}
@Test @Test
fun `register with email token Success should return Success`() = runTest { fun `register with email token Success should return Success`() = runTest {
coEvery { identityService.preLogin(EMAIL) } returns PRE_LOGIN_SUCCESS.asSuccess() coEvery { identityService.preLogin(EMAIL) } returns PRE_LOGIN_SUCCESS.asSuccess()