chore(fix): fixes sync response not correctly identifying all errors (#2346)

2023-07-05 11:28:45 -05:00
parent 2a050fd966
commit 0ab6b5c6fe
19 changed files with 81 additions and 49 deletions
--- a/packages/api/src/Domain/Http/HttpService.ts
+++ b/packages/api/src/Domain/Http/HttpService.ts
@@ -315,12 +315,14 @@ export class HttpService implements HttpServiceInterface {
      console.error(error)
    }
    if (httpStatus >= HttpStatusCode.Success && httpStatus < HttpStatusCode.InternalServerError) {
-      if (
-        httpStatus === HttpStatusCode.Forbidden &&
-        response.data &&
-        (response as HttpErrorResponse).data.error !== undefined
-      ) {
-        ;(response as HttpErrorResponse).data.error.message = ErrorMessage.RateLimited
+      if (httpStatus === HttpStatusCode.Forbidden && isErrorResponse(response)) {
+        if (!response.data.error) {
+          response.data.error = {
+            message: ErrorMessage.RateLimited,
+          }
+        } else {
+          response.data.error.message = ErrorMessage.RateLimited
+        }
      }
      resolve(response)
    } else {
--- a/packages/responses/src/Domain/Error/ClientDisplayableError.ts
+++ b/packages/responses/src/Domain/Error/ClientDisplayableError.ts
@@ -1,4 +1,4 @@
-import { HttpErrorResponse } from '../Http'
+import { HttpErrorResponse, getErrorFromErrorResponse } from '../Http'

 export class ClientDisplayableError {
  constructor(public text: string, public title?: string, public tag?: string) {
@@ -14,7 +14,7 @@ export class ClientDisplayableError {
  }

  static FromNetworkError(error: HttpErrorResponse) {
-    return new ClientDisplayableError(error.data.error.message)
+    return new ClientDisplayableError(getErrorFromErrorResponse(error).message)
  }
 }

--- a/packages/responses/src/Domain/Http/HttpErrorResponseBody.ts
+++ b/packages/responses/src/Domain/Http/HttpErrorResponseBody.ts
@@ -1,5 +1,5 @@
 import { HttpError } from './HttpError'

 export type HttpErrorResponseBody = {
-  error: HttpError
+  error?: HttpError
 }
--- a/packages/responses/src/Domain/Http/HttpResponse.ts
+++ b/packages/responses/src/Domain/Http/HttpResponse.ts
@@ -2,6 +2,7 @@ import { HttpErrorResponseBody } from './HttpErrorResponseBody'
 import { HttpResponseMeta } from './HttpResponseMeta'
 import { HttpHeaders } from './HttpHeaders'
 import { HttpStatusCode } from './HttpStatusCode'
+import { HttpError } from './HttpError'

 type AnySuccessRecord = Record<string, unknown> & { error?: never }

@@ -24,3 +25,14 @@ export type HttpResponse<T = AnySuccessRecord> = HttpErrorResponse | HttpSuccess
 export function isErrorResponse<T>(response: HttpResponse<T>): response is HttpErrorResponse {
  return (response.data as HttpErrorResponseBody)?.error != undefined || response.status >= 400
 }
+
+export function getErrorFromErrorResponse(response: HttpErrorResponse): HttpError {
+  const embeddedError = response.data.error
+  if (embeddedError) {
+    return embeddedError
+  }
+
+  return {
+    message: 'Unknown error',
+  }
+}
--- a/packages/services/src/Domain/AsymmetricMessage/UseCase/GetInboundAsymmetricMessages.ts
+++ b/packages/services/src/Domain/AsymmetricMessage/UseCase/GetInboundAsymmetricMessages.ts
@@ -8,7 +8,7 @@ export class GetInboundAsymmetricMessages {
    const response = await this.messageServer.getMessages()

    if (isErrorResponse(response)) {
-      return ClientDisplayableError.FromError(response.data.error)
+      return ClientDisplayableError.FromNetworkError(response)
    }

    return response.data.messages
--- a/packages/services/src/Domain/AsymmetricMessage/UseCase/GetOutboundAsymmetricMessages.ts
+++ b/packages/services/src/Domain/AsymmetricMessage/UseCase/GetOutboundAsymmetricMessages.ts
@@ -8,7 +8,7 @@ export class GetOutboundAsymmetricMessages {
    const response = await this.messageServer.getOutboundUserMessages()

    if (isErrorResponse(response)) {
-      return ClientDisplayableError.FromError(response.data.error)
+      return ClientDisplayableError.FromNetworkError(response)
    }

    return response.data.messages
--- a/packages/services/src/Domain/AsymmetricMessage/UseCase/SendAsymmetricMessageUseCase.ts
+++ b/packages/services/src/Domain/AsymmetricMessage/UseCase/SendAsymmetricMessageUseCase.ts
@@ -16,7 +16,7 @@ export class SendAsymmetricMessageUseCase {
    })

    if (isErrorResponse(response)) {
-      return ClientDisplayableError.FromError(response.data.error)
+      return ClientDisplayableError.FromNetworkError(response)
    }

    return response.data.message
--- a/packages/services/src/Domain/Files/FileService.ts
+++ b/packages/services/src/Domain/Files/FileService.ts
@@ -386,7 +386,7 @@ export class FileService extends AbstractService implements FilesClientInterface

    const result = await this.api.deleteFile(tokenResult, file.shared_vault_uuid ? 'shared-vault' : 'user')

-    if (result.data?.error) {
+    if (isErrorResponse(result)) {
      const deleteAnyway = await this.alertService.confirm(
        spaceSeparatedStrings(
          'This file could not be deleted from the server, possibly because you are attempting to delete a file item',
@@ -400,7 +400,7 @@ export class FileService extends AbstractService implements FilesClientInterface
      )

      if (!deleteAnyway) {
-        return ClientDisplayableError.FromError(result.data?.error)
+        return ClientDisplayableError.FromNetworkError(result)
      }
    }

--- a/packages/services/src/Domain/Revision/RevisionManager.ts
+++ b/packages/services/src/Domain/Revision/RevisionManager.ts
@@ -1,6 +1,6 @@
 import { RevisionApiServiceInterface } from '@standardnotes/api'
 import { Uuid } from '@standardnotes/domain-core'
-import { isErrorResponse } from '@standardnotes/responses'
+import { getErrorFromErrorResponse, isErrorResponse } from '@standardnotes/responses'

 import { InternalEventBusInterface } from '../Internal/InternalEventBusInterface'
 import { AbstractService } from '../Service/AbstractService'
@@ -21,7 +21,7 @@ export class RevisionManager extends AbstractService implements RevisionClientIn
    const result = await this.revisionApiService.listRevisions(itemUuid.value)

    if (isErrorResponse(result)) {
-      throw new Error(result.data.error.message)
+      throw new Error(getErrorFromErrorResponse(result).message)
    }

    return result.data.revisions
@@ -31,7 +31,7 @@ export class RevisionManager extends AbstractService implements RevisionClientIn
    const result = await this.revisionApiService.deleteRevision(itemUuid.value, revisionUuid.value)

    if (isErrorResponse(result)) {
-      throw new Error(result.data.error.message)
+      throw new Error(getErrorFromErrorResponse(result).message)
    }

    return result.data.message
@@ -41,7 +41,7 @@ export class RevisionManager extends AbstractService implements RevisionClientIn
    const result = await this.revisionApiService.getRevision(itemUuid.value, revisionUuid.value)

    if (isErrorResponse(result)) {
-      throw new Error(result.data.error.message)
+      throw new Error(getErrorFromErrorResponse(result).message)
    }

    return result.data.revision
--- a/packages/services/src/Domain/SharedVaults/UseCase/SendSharedVaultInviteUseCase.ts
+++ b/packages/services/src/Domain/SharedVaults/UseCase/SendSharedVaultInviteUseCase.ts
@@ -23,7 +23,7 @@ export class SendSharedVaultInviteUseCase {
    })

    if (isErrorResponse(response)) {
-      return ClientDisplayableError.FromError(response.data.error)
+      return ClientDisplayableError.FromNetworkError(response)
    }

    return response.data.invite
--- a/packages/services/src/Domain/SharedVaults/UseCase/UpdateSharedVaultInvite.ts
+++ b/packages/services/src/Domain/SharedVaults/UseCase/UpdateSharedVaultInvite.ts
@@ -23,7 +23,7 @@ export class UpdateSharedVaultInviteUseCase {
    })

    if (isErrorResponse(response)) {
-      return ClientDisplayableError.FromError(response.data.error)
+      return ClientDisplayableError.FromNetworkError(response)
    }

    return response.data.invite
--- a/packages/services/src/Domain/Subscription/SubscriptionManager.ts
+++ b/packages/services/src/Domain/Subscription/SubscriptionManager.ts
@@ -4,7 +4,7 @@ import { InternalEventBusInterface } from '../Internal/InternalEventBusInterface
 import { AbstractService } from '../Service/AbstractService'
 import { SubscriptionClientInterface } from './SubscriptionClientInterface'
 import { AppleIAPReceipt } from './AppleIAPReceipt'
-import { isErrorResponse } from '@standardnotes/responses'
+import { getErrorFromErrorResponse, isErrorResponse } from '@standardnotes/responses'

 export class SubscriptionManager extends AbstractService implements SubscriptionClientInterface {
  constructor(
@@ -19,7 +19,7 @@ export class SubscriptionManager extends AbstractService implements Subscription
      const result = await this.subscriptionApiService.acceptInvite(inviteUuid)

      if (isErrorResponse(result)) {
-        return { success: false, message: result.data.error.message }
+        return { success: false, message: getErrorFromErrorResponse(result).message }
      }

      return result.data
@@ -81,7 +81,7 @@ export class SubscriptionManager extends AbstractService implements Subscription
      })

      if (isErrorResponse(result)) {
-        return { success: false, message: result.data.error.message }
+        return { success: false, message: getErrorFromErrorResponse(result).message }
      }

      return result.data
--- a/packages/services/src/Domain/User/UserService.ts
+++ b/packages/services/src/Domain/User/UserService.ts
@@ -1,6 +1,12 @@
 import { Base64String } from '@standardnotes/sncrypto-common'
 import { EncryptionProviderInterface, SNRootKey, SNRootKeyParams } from '@standardnotes/encryption'
-import { HttpResponse, SignInResponse, User, isErrorResponse } from '@standardnotes/responses'
+import {
+  HttpResponse,
+  SignInResponse,
+  User,
+  getErrorFromErrorResponse,
+  isErrorResponse,
+} from '@standardnotes/responses'
 import { KeyParamsOrigination, UserRequestType } from '@standardnotes/common'
 import { UuidGenerator } from '@standardnotes/utils'
 import { UserApiServiceInterface, UserRegistrationResponseBody } from '@standardnotes/api'
@@ -227,7 +233,7 @@ export class UserService
    if (isErrorResponse(response)) {
      return {
        error: true,
-        message: response.data.error.message,
+        message: getErrorFromErrorResponse(response).message,
      }
    }

--- a/packages/snjs/lib/Services/Api/ApiService.ts
+++ b/packages/snjs/lib/Services/Api/ApiService.ts
@@ -664,7 +664,7 @@ export class SNApiService
      })

      if (isErrorResponse(response)) {
-        return ClientDisplayableError.FromError(response.data.error)
+        return ClientDisplayableError.FromNetworkError(response)
      }
      const data = response.data
      return {
--- a/packages/snjs/lib/Services/Session/SessionManager.ts
+++ b/packages/snjs/lib/Services/Session/SessionManager.ts
@@ -45,6 +45,7 @@ import {
  ChangeCredentialsResponse,
  SessionListResponse,
  HttpSuccessResponse,
+  getErrorFromErrorResponse,
 } from '@standardnotes/responses'
 import { CopyPayloadWithContentOverride, RootKeyWithKeyPairsInterface } from '@standardnotes/models'
 import { LegacySession, MapperInterface, Result, Session, SessionToken } from '@standardnotes/domain-core'
@@ -316,7 +317,7 @@ export class SNSessionManager
    const result = await this.apiService.getSubscription(this.getSureUser().uuid)

    if (isErrorResponse(result)) {
-      return ClientDisplayableError.FromError(result.data?.error)
+      return ClientDisplayableError.FromNetworkError(result)
    }

    const subscription = result.data.subscription
@@ -328,7 +329,7 @@ export class SNSessionManager
    const response = await this.apiService.getAvailableSubscriptions()

    if (isErrorResponse(response)) {
-      return ClientDisplayableError.FromError(response.data.error)
+      return ClientDisplayableError.FromNetworkError(response)
    }

    return response.data
@@ -418,8 +419,8 @@ export class SNSessionManager
      ephemeral,
    })

-    if ('error' in registerResponse.data) {
-      throw new ApiCallError(registerResponse.data.error.message)
+    if (isErrorResponse(registerResponse)) {
+      throw new ApiCallError(getErrorFromErrorResponse(registerResponse).message)
    }

    await this.handleAuthentication({
@@ -492,8 +493,8 @@ export class SNSessionManager
    const result = await this.performSignIn(email, password, strict, ephemeral, minAllowedVersion)
    if (
      isErrorResponse(result.response) &&
-      result.response.data.error.tag !== ErrorTag.ClientValidationError &&
-      result.response.data.error.tag !== ErrorTag.ClientCanceledMfa
+      getErrorFromErrorResponse(result.response).tag !== ErrorTag.ClientValidationError &&
+      getErrorFromErrorResponse(result.response).tag !== ErrorTag.ClientCanceledMfa
    ) {
      const cleanedEmail = cleanedEmailString(email)
      if (cleanedEmail !== email) {
--- a/packages/snjs/lib/Services/Settings/SettingsGateway.ts
+++ b/packages/snjs/lib/Services/Settings/SettingsGateway.ts
@@ -1,7 +1,7 @@
 import { SettingsList } from './SettingsList'
 import { SettingName } from '@standardnotes/settings'
 import { API_MESSAGE_INVALID_SESSION } from '@standardnotes/services'
-import { HttpStatusCode, isErrorResponse, User } from '@standardnotes/responses'
+import { getErrorFromErrorResponse, HttpStatusCode, isErrorResponse, User } from '@standardnotes/responses'
 import { SettingsServerInterface } from './SettingsServerInterface'

 /**
@@ -34,7 +34,7 @@ export class SettingsGateway {
    const response = await this.settingsApi.listSettings(this.userUuid)

    if (isErrorResponse(response)) {
-      throw new Error(response.data?.error.message)
+      throw new Error(getErrorFromErrorResponse(response).message)
    }

    if (response.data == undefined || response.data.settings == undefined) {
@@ -53,7 +53,7 @@ export class SettingsGateway {
    }

    if (isErrorResponse(response)) {
-      throw new Error(response.data?.error.message)
+      throw new Error(getErrorFromErrorResponse(response).message)
    }

    return response?.data?.setting?.value ?? undefined
@@ -71,7 +71,7 @@ export class SettingsGateway {
    }

    if (isErrorResponse(response)) {
-      throw new Error(response.data?.error.message)
+      throw new Error(getErrorFromErrorResponse(response).message)
    }

    return response?.data?.setting?.value ?? undefined
@@ -89,7 +89,7 @@ export class SettingsGateway {
    }

    if (isErrorResponse(response)) {
-      throw new Error(response.data?.error.message)
+      throw new Error(getErrorFromErrorResponse(response).message)
    }

    return response.data?.success ?? false
@@ -98,14 +98,14 @@ export class SettingsGateway {
  async updateSetting(name: SettingName, payload: string, sensitive: boolean): Promise<void> {
    const response = await this.settingsApi.updateSetting(this.userUuid, name.value, payload, sensitive)
    if (isErrorResponse(response)) {
-      throw new Error(response.data?.error.message)
+      throw new Error(getErrorFromErrorResponse(response).message)
    }
  }

  async deleteSetting(name: SettingName): Promise<void> {
    const response = await this.settingsApi.deleteSetting(this.userUuid, name.value)
    if (isErrorResponse(response)) {
-      throw new Error(response.data?.error.message)
+      throw new Error(getErrorFromErrorResponse(response).message)
    }
  }

--- a/packages/snjs/lib/Services/Sync/Account/Response.ts
+++ b/packages/snjs/lib/Services/Sync/Account/Response.ts
@@ -9,6 +9,7 @@ import {
  RawSyncResponse,
  UserEventServerHash,
  AsymmetricMessageServerHash,
+  getErrorFromErrorResponse,
 } from '@standardnotes/responses'
 import {
  FilterDisallowedRemotePayloadsAndMap,
@@ -35,8 +36,6 @@ export class ServerSyncResponse {
  private successResponseData: RawSyncResponse | undefined

  constructor(public rawResponse: HttpResponse<RawSyncResponse>) {
-    this.rawResponse = rawResponse
-
    if (!isErrorResponse(rawResponse)) {
      this.successResponseData = rawResponse.data
    }
@@ -101,7 +100,11 @@ export class ServerSyncResponse {
  }

  public get error(): HttpError | undefined {
-    return isErrorResponse(this.rawResponse) ? this.rawResponse.data?.error : undefined
+    if (isErrorResponse(this.rawResponse)) {
+      return getErrorFromErrorResponse(this.rawResponse)
+    } else {
+      return undefined
+    }
  }

  public get status(): number {
@@ -123,6 +126,6 @@ export class ServerSyncResponse {
  }

  public get hasError(): boolean {
-    return this.error != undefined
+    return isErrorResponse(this.rawResponse)
  }
 }
--- a/packages/web/src/javascripts/Components/AccountMenu/SignIn.tsx
+++ b/packages/web/src/javascripts/Components/AccountMenu/SignIn.tsx
@@ -12,7 +12,7 @@ import Icon from '@/Components/Icon/Icon'
 import IconButton from '@/Components/Button/IconButton'
 import AdvancedOptions from './AdvancedOptions'
 import HorizontalSeparator from '../Shared/HorizontalSeparator'
-import { isErrorResponse } from '@standardnotes/snjs'
+import { getErrorFromErrorResponse, isErrorResponse } from '@standardnotes/snjs'

 type Props = {
  viewControllerManager: ViewControllerManager
@@ -99,7 +99,7 @@ const SignInPane: FunctionComponent<Props> = ({ application, viewControllerManag
      .signIn(email, password, isStrictSignin, isEphemeral, shouldMergeLocal)
      .then((response) => {
        if (isErrorResponse(response)) {
-          throw new Error(response.data?.error.message)
+          throw new Error(getErrorFromErrorResponse(response).message)
        }
        viewControllerManager.accountMenuController.closeAccountMenu()
      })
--- a/packages/web/src/javascripts/Components/SessionsModal/SessionsModal.tsx
+++ b/packages/web/src/javascripts/Components/SessionsModal/SessionsModal.tsx
@@ -1,5 +1,12 @@
 import { ViewControllerManager } from '@/Controllers/ViewControllerManager'
-import { SNApplication, SessionStrings, UuidString, SessionListEntry, isErrorResponse } from '@standardnotes/snjs'
+import {
+  SNApplication,
+  SessionStrings,
+  UuidString,
+  SessionListEntry,
+  isErrorResponse,
+  getErrorFromErrorResponse,
+} from '@standardnotes/snjs'
 import { FunctionComponent, useState, useEffect, useRef, useMemo } from 'react'
 import { WebApplication } from '@/Application/WebApplication'
 import { observer } from 'mobx-react-lite'
@@ -64,9 +71,10 @@ function useSessions(
    if (!response) {
      setSessions(sessionsBeforeRevoke)
    } else if (isErrorResponse(response)) {
-      if (response.data?.error.message) {
-        setErrorMessage(response.data?.error.message || 'An unknown error occured while revoking the session.')
-      }
+      setErrorMessage(
+        getErrorFromErrorResponse(response).message || 'An unknown error occured while revoking the session.',
+      )
+
      setSessions(sessionsBeforeRevoke)
    } else {
      setSessions(sessions.filter((session) => session.uuid !== uuid))