[Schema Registry Avro] Remove undefined checks #17714
Conversation
| @@ -202,11 +199,6 @@ export class SchemaRegistryAvroSerializer { | |||
| id = (await this.registry.registerSchema(description)).id; | |||
| } else { | |||
| const response = await this.registry.getSchemaProperties(description); | |||
| if (!response) { | |||
| throw new Error( | |||
| `Schema '${description.name}' not found in registry group '${description.groupName}', or not found to have matching content.` | |||
There was a problem hiding this comment.
I agree we need to remove these checks, but I wonder what the dev experience looks like now.
Is the exception that gets bubbled up as easy to understand? Or might it be worth try/catching and throwing some error with this message and the original one as an innerError or something? (Feel free to file an issue for this instead of handling it in this PR)
| return mapByContent.get(schema.content); | ||
| ): Promise<SchemaProperties> { | ||
| const result = mapByContent.get(schema.content); | ||
| if (result !== undefined) { |
There was a problem hiding this comment.
We can use simple truthy for this, can't we?
| if (result !== undefined) { | |
| if (result) { |
| if (result !== undefined) { | ||
| return result; | ||
| } | ||
| throw new Error(`Schema ID not found: ${id}`); |
There was a problem hiding this comment.
I wonder if inverting the checks would look nicer
| if (result !== undefined) { | |
| return result; | |
| } | |
| throw new Error(`Schema ID not found: ${id}`); | |
| if (!result) { | |
| throw new Error(`Schema ID not found: ${id}`); | |
| } | |
| return result; |
|
Closing this for now since I reverted the change to throw and now the client return |
It makes sense after #17700 was merged in.