Azure-sdk-for-js: Azure schema registry SDK does hava a "schema does not exists" return value

Created on 4 May 2021  路  5Comments  路  Source: Azure/azure-sdk-for-js

Is your feature request related to a problem? Please describe.

When retrieving a schema with a method like SchemaRegistryClient.getSchemaId or SchemaRegistryClient.getSchemaById, an error is thrown if the schema does not exists.

This does not leave me with a clear way to differentiate a functional error (the schema doesn't exists) from a technical errors (like 'there is no network connection' or' you are not authorized to retrieve schemas')

Describe the solution you'd like

The current signature of these 2 methods is:

getSchemaId(schema: SchemaDescription, options?: GetSchemaIdOptions): Promise<SchemaId>;
getSchemaById(id: string, options?: GetSchemaByIdOptions): Promise<Schema>;

A more natural return type would be one that encapsulates functional errors, for example

getSchemaId(schema: SchemaDescription, options?: GetSchemaIdOptions): Promise<SchemaId | null>;
getSchemaById(id: string, options?: GetSchemaByIdOptions): Promise<Schema | null>;

Describe alternatives you've considered

A return type like Promise<Schema | { error: 'notfound', content: '...' }> is more informative, but would not work with the widely used error handling patterns in Javascript like

const id = getSchemaById(...)
if(!id) {
  ...
}

If breaking backwards compatibility is not desired, I'd settle for having a fixed and documented behaviour about how one should check for this error, for example by ensuring the resulting Error in the rejected promise will always have a { statusCode: 404 } field like it does now.

Client Schema Registry customer-reported feature-request needs-team-attention

All 5 comments

@dhoepelman Thanks for your feedback!

@xirzec @ramya-rao-a @bterlson Do we have a standard pattern / guideline in the SDK for how to report "not found"? I feel like all of these suggestions in the issue could work, but I want to be sure we're consistent about it, and maybe there are yet more options from other SDKs of ours to consider.

If breaking backwards compatibility is not desired

Thankfully we're still in preview and can design the right change for the long term without worrying about this. This is why it's great when people give the previews a try so thanks again! :)

Unfortunately, I think it depends a bit on the scenario. Is this something the consumer will regularly to expect to not exist? Then returning undefined/null makes sense (I prefer undefined). But if it's something exceptional, then throwing (rejecting the promise) makes more sense.

In this specific case I'm leaning more towards the Schema | undefined solution.

In this specific case I'm leaning more towards the Schema | undefined solution.

Yeah, I'm leaning that way too. I could see it as not being very exceptional in this case.

15149 is out for review with that approach

Was this page helpful?
0 / 5 - 0 ratings