You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When reviewing code to identify bugs with inviting users to a project, I identified several issues with the translation API that could cause uncaught errors and bugs. I don't think these are related to the invite bug, but I want to ensure they are documented because they could easily cause issues in the future.
Tasks
Uncaught error: The TranslationAPI reads existing translations in the constructor, and throws an error if the getMany() method throws. This is an uncaught error - we should not throw an error here. It should be logged and reported to Sentry (once we set up Sentry for the backend).
Performance: Translations do not make sense for a lot of data types and the only data types that a user can define translations for are preset and field. Attempting to look up translations is expensive, because it is done for every document that is read. Currently we try to translate every document type. We should not even try to translate anything for datatypes other than preset and field, until we actually add support for translating other data types.
Performance: The #translate() method of DataType currently makes two calls to #getTranslations() if regionCode is defined and if there are no translations. regionCode is almost always going to be defined, and there are far more documents without translations that with them. This means that there are lots of unnecessary calls to #getTranslations(). For better performan #getTranslations() should always be called without regionCode defined, and regionCode should used to filter the returned translations, preferring the translation with the regionCode if it exists, but falling back to the translation without the regionCode.
Performance: The cloned translatedDoc is currently untyped, leading to non-type-safe code. Because this is a performance-sensitive area of code, we could just mutate the doc here. If we do clone it, we should only clone it if we are translating the document. Currently we clone every document we read from the database unnecessarily.
Edge-case error: There is currently no check on the value of schemaNamewhen calling #getTranslations() in the DataType class. This was not caught be type checking because of the type issue above. This means that any read of a translation record with lang set (e.g. datatypes.translation.getMany({ lang: 'en' })) will throw an error. This does not happen in current code and should not happen, but we should handle this better.
Edge-case error: The translation datatype has a getTranslations() function that always throws. I don't think this error is handled in the code - there is no error catch in the DataType class for errors when calling this method. We should probably make getTranslation() for the DataType class constructor optional, and not even try to translate datatype records when this is not set.
The text was updated successfully, but these errors were encountered:
Description
When reviewing code to identify bugs with inviting users to a project, I identified several issues with the translation API that could cause uncaught errors and bugs. I don't think these are related to the invite bug, but I want to ensure they are documented because they could easily cause issues in the future.
Tasks
getMany()
method throws. This is an uncaught error - we should not throw an error here. It should be logged and reported to Sentry (once we set up Sentry for the backend).preset
andfield
. Attempting to look up translations is expensive, because it is done for every document that is read. Currently we try to translate every document type. We should not even try to translate anything for datatypes other thanpreset
andfield
, until we actually add support for translating other data types.#translate()
method ofDataType
currently makes two calls to#getTranslations()
ifregionCode
is defined and if there are no translations.regionCode
is almost always going to be defined, and there are far more documents without translations that with them. This means that there are lots of unnecessary calls to#getTranslations()
. For better performan#getTranslations()
should always be called withoutregionCode
defined, and regionCode should used to filter the returned translations, preferring the translation with the regionCode if it exists, but falling back to the translation without the regionCode.translatedDoc
is currently untyped, leading to non-type-safe code. Because this is a performance-sensitive area of code, we could just mutate the doc here. If we do clone it, we should only clone it if we are translating the document. Currently we clone every document we read from the database unnecessarily.schemaName
when calling#getTranslations()
in theDataType
class. This was not caught be type checking because of the type issue above. This means that any read of a translation record withlang
set (e.g.datatypes.translation.getMany({ lang: 'en' })
) will throw an error. This does not happen in current code and should not happen, but we should handle this better.getTranslations()
function that always throws. I don't think this error is handled in the code - there is no error catch in theDataType
class for errors when calling this method. We should probably makegetTranslation()
for the DataType class constructor optional, and not even try to translate datatype records when this is not set.The text was updated successfully, but these errors were encountered: