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

aws-sdk (v2 and v3) response not being decompressed when response is gzip #5266

Closed
lightpriest opened this issue Sep 13, 2023 · 11 comments
Closed
Labels
bug Something isn't working

Comments

@lightpriest
Copy link

What version of Bun is running?

1.0.1-debug+c9c62f37e5bd6362220fb0a39ad20cffd73e486e

What platform is your computer?

Darwin 22.6.0 arm64 arm

What steps can reproduce the bug?

A simple request to AWS SSM using the aws-sdk fails with some cryptic message. The same happens for aws-sdk@3.

The following code generates the issue:

// .mjs file

import AWS from 'aws-sdk';

const ssm = new AWS.SSM({ region: 'eu-central-1' });

console.log(await ssm.getParameter({ Name: "<redacted>" }).promise());

What is the expected behavior?

{
  Parameter: {
    Name: '<redacted>,
    Type: 'String',
    Value: '<redacted>',
    Version: 1,
    LastModifiedDate: <redacted>,
    ARN: '<redacted>',
    DataType: 'text'
  }
}

What do you see instead?

104 |       } else { // synchronous listener
105 |         try {
106 |           listener.apply(self, args);
107 |         } catch (err) {
108 |           // console.error(err);
109 |           error = AWS.util.error(error || new Error(), err);
                                                             ^
SyntaxError: JSON Parse error: Unrecognized token 'ߋ'
      at .../node_modules/aws-sdk/lib/sequential_executor.js:109:58

Additional information

Regarding bun version, it happened to me in bun 1.0.1 as well. I just tried to build the main branch to see if it might have been fixed by now.

I investigated it a bit further and just placed console.log in some specific places. When I just printed the body of the response, it looked like binary data, so I figured it was a compressed response not being deflated.

In the file .../node_modules/aws-sdk/lib/protocol/json.js, in extractData function I modified it to look like this:

function extractData(resp) {
  // +++
  console.log("headers", resp.httpResponse.headers);
  console.log("unzipped body", unzipSync(resp.httpResponse.body).toString('utf8'));
  // ---

  var body = resp.httpResponse.body.toString() || '{}';

  // +++
  console.log("body", resp.httpResponse.body.toString('utf8'));
  // ---

  if (resp.request.service.config.convertResponseTypes === false) {
    resp.data = JSON.parse(body);
  } else {
    var operation = resp.request.service.api.operations[resp.request.operation];
    var shape = operation.output || {};
    var parser = new JsonParser();
    resp.data = parser.parse(body, shape);
  }
}

This validated what I suspected, the output now looks like this:

headers {
  date: "Wed, 13 Sep 2023 12:24:56 GMT",
  "content-type": "application/x-amz-json-1.1",
  "content-length": "186",
  connection: "keep-alive",
  "content-encoding": "gzip",
  server: "Server",
  "x-amzn-requestid": "<redacted>"
}
unzipped body {"Parameter":{"ARN":"<redacted>","DataType":"text","LastModifiedDate":<redacted>,"Name":"<redacted>","Type":"String","Value":"<redacted>","Version":1}}
body <redacted binary data>
104 |       } else { // synchronous listener
105 |         try {
106 |           listener.apply(self, args);
107 |         } catch (err) {
108 |           // console.error(err);
109 |           error = AWS.util.error(error || new Error(), err);
                                                             ^
SyntaxError: JSON Parse error: Unrecognized token 'ߋ'
      at .../node_modules/aws-sdk/lib/sequential_executor.js:109:58

Going a little bit further, I looked at src/bun.js/webcore/response.zig Bun__fetch. I placed a log where disable_decompression is assigned, and saw that it was set to false for some reason.

if (options.get(ctx, "decompress")) |decompress| {
    std.log.info("decompress {}", .{decompress});
    if (decompress.isBoolean()) {
        disable_decompression = !decompress.asBoolean();
    } else if (decompress.isNumber()) {
        disable_keepalive = decompress.to(i32) == 0;
    }
}

// info: decompress src.bun.js.bindings.bindings.JSValue.false

This is as far as I got into it, I haven't gotten into the project's internals to understand where these options are being set (is this a JSC implementation underneath?). And I also think it might be specific to my setup, but I'm not sure what. I'm also surprised it's not reported, since it's a pretty common use-case. Anyway, it should probably work regardless (to be node-compatible).

@lightpriest lightpriest added the bug Something isn't working label Sep 13, 2023
@lightpriest lightpriest changed the title aws-sdk (v2 and v3) responses are not being decompressed when response is in gzip. aws-sdk (v2 and v3) responses are not being decompressed when response is in gzip Sep 13, 2023
@lightpriest lightpriest changed the title aws-sdk (v2 and v3) responses are not being decompressed when response is in gzip aws-sdk (v2 and v3) response not being decompressed when response is gzip Sep 13, 2023
@Antlion12
Copy link

@lightpriest I am having this issue as well with @aws-sdk/client-dynamodb calls running under bun.

      at [REDACTED]/node_modules/@aws-sdk/client-dynamodb/dist-cjs/protocols/Aws_json1_0.js:6388:15
      at processTicksAndRejections (:1:2602)

6383 |     }
6384 |     return new protocol_http_1.HttpRequest(contents);
6385 | };
6386 | const parseBody = (streamBody, context) => collectBodyString(streamBody, context).then((encoded) => {
6387 |     if (encoded.length) {
6388 |         return JSON.parse(encoded);
                   ^
SyntaxError: JSON Parse error: Unrecognized token 'ߋ'

@Antlion12
Copy link

Going a little bit further, I looked at src/bun.js/webcore/response.zig Bun__fetch. I placed a log where disable_decompression is assigned, and saw that it was set to false for some reason.

@lightpriest Do you mean to say decompress was set to false (not that disable_decompression was set to false)?

@Antlion12
Copy link

if (options.get(ctx, "decompress")) |decompress| {
    std.log.info("decompress {}", .{decompress});
    if (decompress.isBoolean()) {
        disable_decompression = !decompress.asBoolean();
    } else if (decompress.isNumber()) {
        disable_keepalive = decompress.to(i32) == 0;
    }
}

// info: decompress src.bun.js.bindings.bindings.JSValue.false

Not sure if this is related, but if decompress.isNumber(), then disable_keepalive gets touched, but not disable_decompression. Wondering if this should be disable_decompression = decompress.to(i32) == 0;.

@iidebyo
Copy link
Contributor

iidebyo commented Sep 13, 2023

see #5043, #5057 they look related and a fix has been merged.

@Antlion12
Copy link

It looks like decompress: false has been the default since this PR was merged 2 weeks ago:

PR 4399 was merged but it looks like the MacOS tests were still failing.

@Antlion12
Copy link

Antlion12 commented Sep 13, 2023

see #5043, #5057 they look related and a fix has been merged.

@iidebyo Ah, this is great. Your fix tells the client to stop adding the extraneous accept-encoding: gzip despite that decompressed: false is set. So once your fix is pushed to production, it seems like requests/responses will be uncompressed from now on and the AWS-related libraries should start working again.

@itsTeknas
Copy link

This would be a deal-breaker for me to use bun in production as AWS-SDK is very required in my business logic.
I really like the efforts of the bun team, and I hope we'll have a drop-in replacement for node soon for my next project.

@lightpriest
Copy link
Author

I can confirm its working in the latest main branch (1.0.1+31aec4ebe325982fc0ef27498984b0ad9969162b). I believe #5057 really did fix it, the build I was trying was made before it was merged.

There's a different issue, though. When I print the response of the aws-sdk (v2) resolved value, I get a $response key that is not there when running in node. This might be something related to toString or maybe something else, but it's not relevant to this issue.

@dnakov
Copy link

dnakov commented Sep 14, 2023

It doesn't work on my end with aws-sdk-v3, same revision as yours

17:08:50 ~/dev/bun-aws-sdk-issue ❯ bun --revision
1.0.1+31aec4ebe325982fc0ef27498984b0ad9969162b

Error:

17:08:50 ~/dev/bun-aws-sdk-issue ❯ bun run index.ts
4494 |         "x-amz-target": `DynamoDB_20120810.${operation}`,
4495 |     };
4496 | }
4497 | const parseBody = (streamBody, context) => collectBodyString(streamBody, context).then((encoded) => {
4498 |     if (encoded.length) {
4499 |         return JSON.parse(encoded);
                   ^
SyntaxError: JSON Parse error: Unrecognized token 'ߋ'
  Deserialization error: to see the raw response, inspect the hidden field {error}.$response on this object.
      at /Users/daniel/dev/bun-aws-sdk-issue/node_modules/@aws-sdk/client-dynamodb/dist-cjs/protocols/Aws_json1_0.js:4499:15
      at processTicksAndRejections (:1:2602)

Here's code to reproduce:

import { DynamoDBClient } from '@aws-sdk/client-dynamodb';
import { DynamoDBDocumentClient, ScanCommand } from '@aws-sdk/lib-dynamodb';
const dynamoDb = DynamoDBDocumentClient.from(new DynamoDBClient({ region: 'us-east-1' }));

async function main() {
  const res = await dynamoDb.send(new ScanCommand({
    TableName: 'MyTable'
  }))
  console.log(res)
}

main()

fwiw, I noticed that after bun install, i get the same error when run with node, so I have to delete node_modules and run npm install for it to work in node again, but I can't get it to work with bun at all.

@iidebyo
Copy link
Contributor

iidebyo commented Sep 14, 2023

I think it'd be in a new version. I don't see #5057 in v1.0.1 commits.

https://github.com/oven-sh/bun/commits/bun-v1.0.1

@ShivamJoker
Copy link

I see, this issue was coming from the day 1.
Bun needs to be tested more and more to make it production ready.

I'm glad it has been fixed in the latest release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants