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

Failure to parse json from response for aws_sdk_quicksight::Client::list_data_sources() #1095

Closed
mjc3bb opened this issue Mar 7, 2024 · 7 comments
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@mjc3bb
Copy link

mjc3bb commented Mar 7, 2024

Describe the bug

In aws-models/quicksight.json DataSourceParameters has the union type, but the api appears to return a structure type with nulls for the other variants. Due to this, when calling aws_sdk_quicksight::Client::list_data_sources() it errors due to AmazonElasticsearchParameters being returned as null.

Expected Behavior

Calling aws_sdk_quicksight::Client::list_data_sources() should correctly parse the returned api response and provide a single enum variant of DataSourceParameters.

Current Behavior

The api appears to return a structure type instead of a union type, so it has a null value for all possible variants, except the variant it actually is. The generated code bounces off of the first variant check which is AmazonElasticsearchParameters.

https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/quicksight/src/protocol_serde/shape_data_source_parameters.rs#L195

body of datasourceparameters from the trace logs:

{
  "AmazonElasticsearchParameters": null,
  "AmazonOpenSearchParameters": null,
  "AppFlowParameters": null,
  "AthenaParameters": null,
  "AuroraParameters": null,
  "AuroraPostgreSqlParameters": null,
  "AwsIotAnalyticsParameters": null,
  "BigQueryParameters": null,
  "DatabricksParameters": null,
  "Db2Parameters": null,
  "DenodoParameters": null,
  "DocumentDBParameters": null,
  "DremioParameters": null,
  "ExasolParameters": null,
  "GoogleAnalyticsParameters": null,
  "JiraParameters": null,
  "MariaDbParameters": null,
  "MongoAtlasParameters": null,
  "MongoDBParameters": null,
  "MySqlParameters": null,
  "OracleParameters": null,
  "PostgreSqlParameters": null,
  "PrestoParameters": null,
  "RdsParameters": null,
  "RedshiftParameters": null,
  "S3Parameters": {
    "IsUploaded": false,
    "ManifestFileLocation": {
      "Bucket": "deided-bucket.prod.us-east-1",
      "Key": "sales/manifest.json"
    },
    "RoleArn": null
  },
  "SalesforceParameters": null,
  "SapHanaParameters": null,
  "ServiceNowParameters": null,
  "SnowflakeParameters": null,
  "SparkParameters": null,
  "SqlServerParameters": null,
  "StarburstParameters": null,
  "TeradataParameters": null,
  "TrinoParameters": null,
  "TwitterParameters": null
}

stdout from test to reproduce issue

---- backup::reproduce_error stdout ----
thread 'backup::reproduce_error' panicked at src/backup.rs:175:47:
called `Result::unwrap()` on an `Err` value: service error

Caused by:
    0: unhandled error
    1: failed to parse JSON: value for 'AmazonElasticsearchParameters' cannot be null

Reproduction Steps

let config = aws_config::defaults(aws_config::BehaviorVersion::latest()).load.await;
let client = aws_sdk_quicksight::Client::new(&config);
let resp = client.list_data_sources().aws_account_id("<id here>").send.await;

resp will be a service error about failing to parse json.

Possible Solution

A quick fix that I validated works is to use a structure type instead of a union type in the aws-model, like boto3 does. This severely limits the usability of the type however since you can no longer match against it to determine which DataSourceParameters variant it used. It's possible this is more of a QuickSight api problem, as it returns surprising/unexpected data.

Additional Information/Context

No response

Version

├── aws-config v1.1.7
│   ├── aws-credential-types v1.1.7
│   │   ├── aws-smithy-async v1.1.7
│   │   ├── aws-smithy-runtime-api v1.1.7
│   │   │   ├── aws-smithy-async v1.1.7 (*)
│   │   │   ├── aws-smithy-types v1.1.7
│   │   ├── aws-smithy-types v1.1.7 (*)
│   ├── aws-runtime v1.1.7
│   │   ├── aws-credential-types v1.1.7 (*)
│   │   ├── aws-sigv4 v1.1.7
│   │   │   ├── aws-credential-types v1.1.7 (*)
│   │   │   ├── aws-smithy-http v0.60.6
│   │   │   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   │   │   ├── aws-smithy-types v1.1.7 (*)
│   │   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-smithy-async v1.1.7 (*)
│   │   ├── aws-smithy-http v0.60.6 (*)
│   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-types v1.1.7
│   │   │   ├── aws-credential-types v1.1.7 (*)
│   │   │   ├── aws-smithy-async v1.1.7 (*)
│   │   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   │   ├── aws-smithy-types v1.1.7 (*)
│   ├── aws-sdk-sso v1.15.0
│   │   ├── aws-credential-types v1.1.7 (*)
│   │   ├── aws-runtime v1.1.7 (*)
│   │   ├── aws-smithy-async v1.1.7 (*)
│   │   ├── aws-smithy-http v0.60.6 (*)
│   │   ├── aws-smithy-json v0.60.6
│   │   │   └── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-smithy-runtime v1.1.7
│   │   │   ├── aws-smithy-async v1.1.7 (*)
│   │   │   ├── aws-smithy-http v0.60.6 (*)
│   │   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-types v1.1.7 (*)
│   ├── aws-sdk-ssooidc v1.15.0
│   │   ├── aws-credential-types v1.1.7 (*)
│   │   ├── aws-runtime v1.1.7 (*)
│   │   ├── aws-smithy-async v1.1.7 (*)
│   │   ├── aws-smithy-http v0.60.6 (*)
│   │   ├── aws-smithy-json v0.60.6 (*)
│   │   ├── aws-smithy-runtime v1.1.7 (*)
│   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-types v1.1.7 (*)
│   ├── aws-sdk-sts v1.15.0
│   │   ├── aws-credential-types v1.1.7 (*)
│   │   ├── aws-runtime v1.1.7 (*)
│   │   ├── aws-smithy-async v1.1.7 (*)
│   │   ├── aws-smithy-http v0.60.6 (*)
│   │   ├── aws-smithy-json v0.60.6 (*)
│   │   ├── aws-smithy-query v0.60.6
│   │   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-smithy-runtime v1.1.7 (*)
│   │   ├── aws-smithy-runtime-api v1.1.7 (*)
│   │   ├── aws-smithy-types v1.1.7 (*)
│   │   ├── aws-smithy-xml v0.60.6
│   │   ├── aws-types v1.1.7 (*)
│   ├── aws-smithy-async v1.1.7 (*)
│   ├── aws-smithy-http v0.60.6 (*)
│   ├── aws-smithy-json v0.60.6 (*)
│   ├── aws-smithy-runtime v1.1.7 (*)
│   ├── aws-smithy-runtime-api v1.1.7 (*)
│   ├── aws-smithy-types v1.1.7 (*)
│   ├── aws-types v1.1.7 (*)
├── aws-sdk-quicksight v1.17.0
│   ├── aws-credential-types v1.1.7 (*)
│   ├── aws-runtime v1.1.7 (*)
│   ├── aws-smithy-async v1.1.7 (*)
│   ├── aws-smithy-http v0.60.6 (*)
│   ├── aws-smithy-json v0.60.6 (*)
│   ├── aws-smithy-runtime v1.1.7 (*)
│   ├── aws-smithy-runtime-api v1.1.7 (*)
│   ├── aws-smithy-types v1.1.7 (*)
│   ├── aws-types v1.1.7 (*)


### Environment details (OS name and version, etc.)

macOS Ventura 13.6 and Pop!_OS 22.04 LTS

### Logs

_No response_
@mjc3bb mjc3bb added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 7, 2024
@rcoh
Copy link
Contributor

rcoh commented Mar 12, 2024

seems like either a parsing bug or a service side bug. We're investigating and will get this fixed

@rcoh rcoh added p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Mar 12, 2024
@rcoh
Copy link
Contributor

rcoh commented Mar 12, 2024

Bug on our side. Fixed here: smithy-lang/smithy-rs#3481 It just missed the train for the new release of the code generator but it will go out in the next one or a patch.

@mjc3bb
Copy link
Author

mjc3bb commented Mar 13, 2024

Thanks so much for the quick response and fix on this!

github-merge-queue bot pushed a commit to smithy-lang/smithy-rs that referenced this issue Mar 15, 2024
## Motivation and Context
- awslabs/aws-sdk-rust#1095

## Description
Update the JSON parser generator to allow for `null` to be set in
unions. Servers can send unions like this:
```json
{
  "AmazonElasticsearchParameters": null,
  "AmazonOpenSearchParameters": null,
  "AppFlowParameters": null,
  "AthenaParameters": null,
  "AuroraParameters": null,
  "AuroraPostgreSqlParameters": null,
  "AwsIotAnalyticsParameters": null,
  "BigQueryParameters": null,
  "DatabricksParameters": null,
  "Db2Parameters": null,
  "DenodoParameters": null,
  "DocumentDBParameters": null,
  "DremioParameters": null,
  "ExasolParameters": null,
  "GoogleAnalyticsParameters": null,
  "JiraParameters": null,
  "MariaDbParameters": null,
  "MongoAtlasParameters": null,
  "MongoDBParameters": null,
  "MySqlParameters": null,
  "OracleParameters": null,
  "PostgreSqlParameters": null,
  "PrestoParameters": null,
  "RdsParameters": null,
  "RedshiftParameters": null,
  "S3Parameters": {
    "IsUploaded": false,
    "ManifestFileLocation": {
      "Bucket": "deided-bucket.prod.us-east-1",
      "Key": "sales/manifest.json"
    },
    "RoleArn": null
  },
  "SalesforceParameters": null,
  "SapHanaParameters": null,
  "ServiceNowParameters": null,
  "SnowflakeParameters": null,
  "SparkParameters": null,
  "SqlServerParameters": null,
  "StarburstParameters": null,
  "TeradataParameters": null,
  "TrinoParameters": null,
  "TwitterParameters": null
}
```

This caused our parser to fail.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
- [x] New unit test
- [x] Dry run against new [smithy protocol
test](smithy-lang/smithy#2180)

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@mjc3bb
Copy link
Author

mjc3bb commented Mar 22, 2024

Hi, just wanted to follow up on this issue. It wasn't high priority at the time I submitted this issue but it is starting to become a blocker for a project at work. I don't see a listed cadence, is there any expected timeline for a new release to be cut for smithy-rs so that this change can make it into aws-sdk-rust?

Edit: I see now the changes actually are in next, so hopefully that means we can expect a release with those changes relatively soon?

@jdisanti
Copy link
Contributor

The fix was merged into smithy-rs, so it'll go out the next time we do a smithy-rs release, which should be pretty soon.

@ysaito1001
Copy link
Collaborator

Confirmed the reproduction steps were executed successfully with aws-sdk-quicksight of version 1.26.0 (must've been fixed in an earlier version).

Copy link

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

4 participants