Not clear how to narrow `BuildModelDefaultResponse | BuildModelLogicalResponse` when using `await poller.pollUntilDone()`

nicu-chiciuc commented 3 weeks ago

Is your feature request related to a problem? Please describe. Based on the documentation regarding the migration to '@azure-rest/ai-document-intelligence' from '@azure/ai-form-recognizer'. https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/documentintelligence/ai-document-intelligence-rest/MIGRATION-FR_v4-DI_v1.md

I do something like this:

  const poller = await getLongRunningPoller(client, initialResponse)
  const response = await poller.pollUntilDone()

  const analyzeResult = response.body?.analyzeResult

The error appears since response has a type of BuildModelDefaultResponse | BuildModelLogicalResponse.

These are the definitions

export declare interface BuildModelDefaultResponse extends HttpResponse {
    status: string;
    body: ErrorResponseOutput;
}

/** The final response for long-running buildModel operation */
export declare interface BuildModelLogicalResponse extends HttpResponse {
    status: "200";
}

I assume the idea here is that the client can do something like

if (response.status === '200') {
// response is narrowed to BuildModelLogicalResponse
}

But that doesn't work.

The only solution I've seen in the docs is to use type assertions. But that breaks type-safety.

Describe the solution you'd like I would like to know how to narrow the type of the response to BuildModelLogicalResponse

Describe alternatives you've considered This can be overcome using // @ts-expect-error or something similar.

Additional context Done above.

xirzec commented 3 weeks ago

My understanding is that you can disambiguate by doing something like

if (isUnexpected(initialResponse)) {
  throw initialResponse.body.error;
}

Since isUnexpected will take care of the default/error case and narrow to the successful result.

HarshaNalluru commented 3 weeks ago

Thanks @xirzec, that should do it.

@nicu-chiciuc thanks for reaching out. You can also refer to examples at https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/documentintelligence/ai-document-intelligence-rest/samples/v1-beta

github-actions[bot] commented 3 weeks ago

Hi @nicu-chiciuc. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

nicu-chiciuc commented 3 weeks ago

@xirzec I also use isUnexpected for the initialResponse. The issue is was about the await poller.pollUntilDone().

Using it for the poller response solves the issue I thought I had (it narrows toBuildModelLogicalResponse), but the body is still unknown.

@HarshaNalluru the example I've seen use type assertion for this:

As can be seen from this search: https://github.com/search?q=repo%3AAzure%2Fazure-sdk-for-js+path%3Adocumentintelligence+path%3Ats+%22pollUntilDone%28%29%22&type=code

There is not a single example that doesn't rely on type assertion.

Currently I use @ts-expect-error since type assertions hide that it's an error:

Here's the full code for the function.

export async function extractUsingAiIntelligenceMarkdown(url: string) {
  const credential = new AzureKeyCredential(envs.AZURE_FORM_RECOGNIZER_KEY)

  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore - This doesn't show an error, but will fail for Next.js since Next.js imports this project !?!
  const client = DocumentIntelligence.default(envs.AZURE_FORM_RECOGNIZER_ENDPOINT, credential)

  const initialResponse = await client.path('/documentModels/{modelId}:analyze', 'prebuilt-layout').post({
    contentType: 'application/json',
    body: {
      urlSource: url,
    },
    queryParameters: { outputContentFormat: 'markdown' }, // <-- new query parameter
  })

  if (isUnexpected(initialResponse)) {
    throw initialResponse.body.error
  }

  const poller = await getLongRunningPoller(client, initialResponse)
  const response: BuildModelDefaultResponse | BuildModelLogicalResponse = await poller.pollUntilDone()

  // @ts-expect-error https://github.com/Azure/azure-sdk-for-js/issues/30019
  const forcedResponse: GetAnalyzeResult200Response = response

  const content = forcedResponse.body?.analyzeResult?.content ?? ''

  return content
}

Relying on type assertion is not type safe and can/will introduce errors.

Azure / azure-sdk-for-js

Not clear how to narrow `BuildModelDefaultResponse | BuildModelLogicalResponse` when using `await poller.pollUntilDone()` #30019