Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nicu-chiciuc opened this issue Jun 12, 2024 · 4 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Form Recognizer customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@nicu-chiciuc
Copy link

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
Screenshot 2024-06-12 at 14 39 15

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

Screenshot 2024-06-12 at 14 39 31

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.

@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jun 12, 2024
@github-actions github-actions bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 12, 2024
@xirzec xirzec added the Client This issue points to a problem in the data-plane of the library. label Jun 12, 2024
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jun 12, 2024
@xirzec
Copy link
Member

xirzec commented Jun 12, 2024

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
Copy link
Member

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

@xirzec xirzec added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Jun 13, 2024
@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jun 13, 2024
Copy link

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
Copy link
Author

@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.
Screenshot 2024-06-14 at 12 49 25

@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.

@xirzec xirzec removed the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Jun 14, 2024
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Cognitive - Form Recognizer customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

4 participants