goshippo / shippo-javascript-sdk

MIT License
5 stars 0 forks source link

customsDeclaration.create returning zod errors when using previously valid customsItems.create objects #17

Closed alsoscotland closed 1 month ago

alsoscotland commented 2 months ago

In version v2.1.2 (attempting upgrade from 1.7.1)

When using the client.customsDeclarations.create method. I used to be able to pass an array of customsItems objectIds in the items field. These were successfully created using the the client.customsItems.create method.

In the latest version, the type CustomsDeclarationCreateRequest has the items property typed as items: Array<CustomsItemCreateRequest>;

When I send the same configuration objects which previously worked using customsItems.create, several zod typing errors are returned.

example request

 {
  "contentsType": "MERCHANDISE",
  "contentsExplanation": "{example} merchandise",
  "nonDeliveryOption": "RETURN",
  "certify": true,
  "certifySigner": "Example User",
  "items": [
    {
      "description": "{example} merchandise",
      "quantity": 1,
      "netWeight": "8",
      "massUnit": "oz",
      "valueAmount": "159",
      "valueCurrency": "USD",
      "originCountry": "US"
    }
  ]
}

errors returned from await client.customsDeclarations.create(exampleRequest)

{
    "status": "rejected",
    "reason": {
    "writeToLog": false,
    "originalError": {
        "name": "SDKValidationError",
        "cause": {
        "issues": [
            {
            "received": "",
            "code": "invalid_enum_value",
            "options": [
                "FILED_ELECTRONICALLY",
                "SUMMARY_REPORTING",
                "NOT_REQUIRED"
            ],
            "path": [
                "b13a_filing_option"
            ],
            "message": "Invalid enum value. Expected 'FILED_ELECTRONICALLY' | 'SUMMARY_REPORTING' | 'NOT_REQUIRED', received ''"
            },
            {
            "code": "invalid_type",
            "expected": "string",
            "received": "boolean",
            "path": [
                "commercial_invoice"
            ],
            "message": "Expected string, received boolean"
            },
            {
            "received": "",
            "code": "invalid_enum_value",
            "options": [
                "NOEEI_30_37_a",
                "NOEEI_30_37_h",
                "NOEEI_30_37_f",
                "NOEEI_30_36",
                "AES_ITN"
            ],
            "path": [
                "eel_pfc"
            ],
            "message": "Invalid enum value. Expected 'NOEEI_30_37_a' | 'NOEEI_30_37_h' | 'NOEEI_30_37_f' | 'NOEEI_30_36' | 'AES_ITN', received ''"
            },
            {
            "received": "",
            "code": "invalid_enum_value",
            "options": [
                "DDP",
                "DDU",
                "FCA",
                "DAP",
                "eDAP"
            ],
            "path": [
                "incoterm"
            ],
            "message": "Invalid enum value. Expected 'DDP' | 'DDU' | 'FCA' | 'DAP' | 'eDAP', received ''"
            }
        ],
        "name": "ZodError"
        },
        "rawValue": {
        "object_created": "2024-05-06T03:21:14.498Z",
        "object_updated": "2024-05-06T03:21:14.498Z",
        "object_id": "5599a71217df4977b3185499ae8bb11d",
        "object_owner": "exampleuser@users.com",
        "object_state": "VALID",
        "certify_signer": "example user",
        "certify": true,
        "items": [
            "e761308d66684857a6cc3a2e3b3fd3f6"
        ],
        "non_delivery_option": "RETURN",
        "contents_type": "MERCHANDISE",
        "contents_explanation": "example merchandise",
        "exporter_reference": "",
        "importer_reference": "",
        "invoice": "",
        "commercial_invoice": false,
        "license": "",
        "certificate": "",
        "notes": "",
        "eel_pfc": "",
        "aes_itn": "",
        "disclaimer": "",
        "incoterm": "",
        "metadata": "",
        "test": true,
        "b13a_filing_option": "",
        "b13a_number": "",
        "is_vat_collected": false
        }
    },
    "type": "shipping_transaction_processing",
    "status": 400
    }
}

They seem to be primarily issues with zod enum values. Interestingly, an objectId does seem to have been generated for the customsItem.

Please advise on whether this behavior is a bug or if there are reasonable defaults to use for all of these enums.

shippo-lueders commented 2 months ago

this is a known issue and is something I will be picking up this week. basically, we are returning an empty string for some enum fields and an enum is a closed set - and an empty string is not a valid return value.

the problem is not with the input, it's with the output. the request actually succeeds (hence, the objectId), but fails during deserialization of the response.

alsoscotland commented 2 months ago

@shippo-lueders Thank you for the update. Will keep an eye out for the next version

shippo-lueders commented 1 month ago

@alsoscotland https://www.npmjs.com/package/shippo/v/2.2.3 has been published which should resolve this issue.

sorry it took so long, the issue of enums is a bit thorny. in this particular case, we're returning an empty string for enums which we really shouldn't be doing but there are very possibly clients depending on this behavior, so we can't alter that behavior without likely breaking some clients. more generally, we have fields represented as enums (e.g. service level) which will absolutely vary over time. the solution we've opted for is to separate the models based on whether the schema is used for input or output. for input schemas, the field is an enum; for output schemas, the field is a string. the rationale being, enums are valuable for input models b/c they offer code completion in IDEs.

I've only done this for the customs models so far but it's likely we'll make this approach more ubiquitous. the other (and simpler) option would be to eliminate enums completely from fields and just use string, where the enum is still available as a schema object but just not directly referenced as a field type. internally, this is simpler; but the belief is that this would lead to a worse user experience. as a user of the SDK, any thoughts you have on the subject are appreciated.

alsoscotland commented 1 month ago

@shippo-lueders Thank you for the update. For what it is worth I have opted out of using typescript string enums in my API code personally in favor of other options like union types or template literal types. For me, the duality of enums getting exported as both types and values always caused a little confusion for me. It is a topic that seemingly gets debated a ton and has merits from multiple perspectives.

In terms of consumption, I think that this means I now have to import the enum type and reference its properties directly off from the value when calling the APIs. This may be confusing to some people if they are trying to use the string value of the enum member directly when attempting to call the API methods.

I have not tried it but it might be worth investigating something like this for your output types.

type DistanceUnitEnumOutputType =${DistanceUnitEnum}& '';

alsoscotland commented 1 month ago

@shippo-lueders Found this yesterday https://www.youtube.com/watch?v=jjMbPt_H3RQ It does a very good job of illustrating the details around enums