microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
3.02k stars 210 forks source link

Cannot find name 'createStringFromDiscriminatorValue' when generating typescript #4018

Closed sword-jin closed 10 months ago

sword-jin commented 10 months ago
kiota --version
1.10.1+fa5da75b8b4f362d821032021bc5bfcf063cb522

openapi yaml like:

openapi: '3.0.2'
info:
  title: JSONPlaceholder
  version: '1.0'
servers:
  - url: https://jsonplaceholder.typicode.com/

paths:
  /api/download/{blobId}:
    parameters:
    - in: path
      name: blobId
      schema:
        type: string
      required: true
      description: BLOB ID
    get:
      summary: download temporary BLOB
      responses:
        '302':
          description: Found the BLOB and returned a signed URL to download it.
          headers:
            Location:
              required: true
              schema:
                type: string
        '400':
          description: Invalid ID
        '403':
          description: Not allowed
        '404':
          description: not found.
        default:
          description: General Error
          content:
            application/json:
              schema:
                type: string

the generated code is using createStringFromDiscriminatorValue which is not imported and not found in the item/index.ts

    public get(requestConfiguration?: RequestConfiguration<object> | undefined) : Promise<ArrayBuffer | undefined> {
        const requestInfo = this.toGetRequestInformation(
            requestConfiguration
        );
        const errorMapping = {
            "4XX": createStringFromDiscriminatorValue,
            "5XX": createStringFromDiscriminatorValue,
        } as Record<string, ParsableFactory<Parsable>>;
        return this.requestAdapter.sendPrimitiveAsync<ArrayBuffer>(requestInfo, "ArrayBuffer", errorMapping);
    }

I guess the correct one is createMessageFromDiscriminatorValue

baywet commented 10 months ago

Hi @sword-jin, Thanks for using kiota and for reaching out. If you remove the "default" section, do you still get the error mappings generated? Also kiota clients support automatic redirection, so instead of describing the 302, you could describe the result you get after the redirection and have the client follow it automatically.

sword-jin commented 10 months ago

@baywet Yes, it disappeared. We need to use this definition to generate server code and the 302 is required. So, is this a bug?

baywet commented 10 months ago

The error mapping is most likely a bug yes. The 302 can stay as is, it was simply in case the actual payload is schematize if you wanted kiota to generate type for it.

For the error mapping issue, would you be willing to submit a pull request?

This is most likely caused by the fact that these conditions are not properly built:

https://github.com/microsoft/kiota/blob/95a6c2ad941f2017a457adc259cf85134638fd90/src/Kiota.Builder/KiotaBuilder.cs#L1293

The call to AddErrorMapping should be in the first if block (where IsErrorDefinition = true; is) The LogWarning should be in an else block instead.

And you can add a unit test by copying this one

https://github.com/microsoft/kiota/blob/95a6c2ad941f2017a457adc259cf85134638fd90/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs#L1954

sword-jin commented 10 months ago

schematize

After I changed the logic like you described,

            {
                codeClass.IsErrorDefinition = true;
                executorMethod.AddErrorMapping(errorCode, errorType);
            }
            if (errorType != null)
                logger.LogWarning("Could not create error type for {Error} in {Operation}", errorCode, operation.OperationId);

there is no errorMapping in the generated code. Is that right? I am not familiar with c#, please help to fix it if you have time. @baywet

baywet commented 10 months ago

no worries, just authored microsoft/kiota#4026 to address this issue.

sword-jin commented 10 months ago

I see, please inform me after you create a new tag, I will test it by latest docker image.

baywet commented 10 months ago

On the nightly tag we have a preview going out every Thursday afternoon, Eastern Time.

sword-jin commented 10 months ago

@baywet , why didn't push a release last week? I am waiting for this bug fix

baywet commented 10 months ago

@sword-jin we had a code-sign certificate expire last week unfortunately. Thank you for your patience while we work to renew it and unblock releases.

sword-jin commented 10 months ago

Please release once manually if it's possible. 🙂

baywet commented 10 months ago

there you go! https://github.com/microsoft/kiota/releases/tag/v1.11.0-preview.202401300001

sword-jin commented 10 months ago

@baywet

I feel there is still a bug here

image

I am using latest pacakges:

  "dependencies": {
    "@microsoft/kiota-abstractions": "^1.0.0-preview.39",
    "@microsoft/kiota-http-fetchlibrary": "^1.0.0-preview.38",
    "@microsoft/kiota-serialization-form": "^1.0.0-preview.28",
    "@microsoft/kiota-serialization-json": "^1.0.0-preview.39",
    "@microsoft/kiota-serialization-multipart": "^1.0.0-preview.18",
    "@microsoft/kiota-serialization-text": "^1.0.0-preview.36"
  }

image

Type '(writer: SerializationWriter, multipartBody: MultipartBody | undefined) => void' is not assignable to type 'PrimitiveTypesForDeserialization | ModelSerializerFunction<Parsable> | undefined'.
  Type '(writer: SerializationWriter, multipartBody: MultipartBody | undefined) => void' is not assignable to type 'ModelSerializerFunction<Parsable>'.
    Types of parameters 'multipartBody' and 'value' are incompatible.
      Type 'Parsable | undefined' is not assignable to type 'MultipartBody | undefined'.
        Type 'Parsable' is missing the following properties from type 'MultipartBody': _boundary, _parts, addOrReplacePart, getPartValue, and 4 more.ts(2322)
(property) RequestMetadata.requestBodySerializer?: PrimitiveTypesForDeserialization | ModelSerializerFunction<Parsable> | undefined
sword-jin commented 10 months ago

Also, there is no errorMapping in the post, get, I expect it has like:

errorMappings: {
            _4XX: createODataErrorFromDiscriminatorValue as ParsableFactory<Parsable>,
            _5XX: createODataErrorFromDiscriminatorValue as ParsableFactory<Parsable>,
        },

I am still using the original openapi.yaml in this issue description.

baywet commented 10 months ago

Can you open a separate issue for the multi part thing please?

Yes this is expected for your description, since the error being described is only a string, kiota can't generate a type that's both a string AND inherits from ApiException (so it can be thrown) We might consider an improvement in the future to read simple strings, and use them as a message, but the goal of the initial PR was simply to unblock you.

sword-jin commented 10 months ago

Sure, for error in this PR https://github.com/microsoft/kiota-typescript/issues/1053