CRC32 check failed, marked as retryable but actually request will not be repeated

[srkimir]

const AWS = require('aws-sdk')
const dynamodb = new AWS.DynamoDB({
  accessKeyId: '...',
  secretAccessKey: '...',
  region: '...'
})

dynamodb.listTables({}, function(err, data) {
  if (err) {
    console.log(err)
  } else {
    console.log(data)
  }
})

Will produce this error:

{ 
  [CRC32CheckFailed: CRC32 integrity check failed]
  message: 'CRC32 integrity check failed',
  code: 'CRC32CheckFailed',
  retryable: true,
  time: Sun May 01 2016 23:20:52 GMT+0300 (EEST),
  statusCode: 200 
}

In the first place why CRC32CheckFailed happens at all? Second, as far as i know maxRetries defaults to 10, when CRC32CheckFailed happens it will return statusCode equal to 200 with retryable: true, but i don't see any repeated requests. Shouldn't request be repeated?

Third, most important, even if(err) resolve to true, second parameter, data is also defined and actually it will return good results. How is that possible? That should never happen if err exists

[LiuJoyceC]

Hi @srkimir
Can you provide more specific code to replicate the error? The error is arising because dynamodb.crc32IsValid() is returning false, but without further information about your specific case, I could not reproduce this error. Also, when I forced the error to fail by returning false from crc32IsValid, the request was successfully retried 10 times before passing back the CRC32CheckFailed error into my callback, with an empty object {} passed into the data parameter.

Also, just to get more information about your case, what version of the SDK and what version of Node are you using?

[srkimir]

@LiuJoyceC Problem happens occasionally, code example that i have posted sometimes yields CRC32CheckFailed and error and sometimes not. I guess only way to replicate this error is to force same call over and over.

Im using Node v4.4.3 and AWS-SDK API Version 2012-08-10

passing back the CRC32CheckFailed error into my callback, with an empty object {} passed into the data parameter.

If err exists, no matter how you force it, you should never have data defined at all in that case

[chrisradek]

@srkimir
A crc32 check(a) is done on the body of a response to verify that the the response contains the correct data that DynamoDB meant to send. One way a discrepancy could occur is if there were network issues that caused only part of the response to be sent, which would lead to a JSON parse failure.

When a crc32 check fails, the request should be retried, up to 10 times by default. How are you checking that a request is repeated? The callback you pass to listTables will only be called once, regardless of how many times the request is retried. You can verify requests are being retried by logging them out.

const dynamodb = new AWS.DynamoDB({
  accessKeyId: '...',
  secretAccessKey: '...',
  region: '...',
  logger: console
});

I agree with you that data should be null when an error exists and we need to investigate why for this service, that's not the case.

You posted the API version of the DynamoDB client you're using, can you also post what version of the SDK you're using? One way to find out is by logging AWS.VERSION from within an application.

a. https://github.com/aws/aws-sdk-js/blob/master/lib/services/dynamodb.js#L19

[srkimir]

@chrisradek thank you on you response
AWS.version returns 2.2.48

I didn't check that request was repeated, i will try to force error again and to check it.
Im a little bit confused about statusCode: 200 that i received back.
From http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/ErrorHandling.html i would expect that few errors from 4xx and all from 5xx will be repeated. To me it seems that AWS-SDK consider CRC32CheckFailed as successful request so it will not repeat it, right? Having retryable: true and statusCode: 200 it seems the two are contradictory

[chrisradek]

@srkimir
The crc32 check is a special case where the SDK will also retry the error, even if the status code is 200. The status code is 200 because from the service's perspective, it was able to properly send a response. On the client side, the data integrity check (crc32 in this case) indicates that something went wrong, such as a networking issue when the response was already in flight, so it will retry the request.

I hope that clarifies the behavior you're seeing!

[srkimir]

@chrisradek thanks on very good explanation

I agree with you that data should be null when an error exists and we need to investigate why for this service, that's not the case.

Only thing that left unclear and it should be fixed

[LiuJoyceC]

The data was set to an empty object in the 'validate' step of the ASM, since a successful response came back from the server, so it is not the service that is returning back the non-null data. The checkCrc32 validation doesn't occur until the 'extractData' step after the ASM already determined that the response was successful. This bug can be fixed by either setting data back to null each time checkCrc32 fails or by moving that check to the 'validate' step of the ASM. The latter seems more correct, so I will investigate whether it might cause any breaking changes or unintended effects before implementing it. If it does cause undesirable effects, then I will implement the former fix.

@srkimir Were you able to verify if your request is being retried? Thanks

[LiuJoyceC]

Upon further investigation, it's correct to keep checkCrc32 in the 'extractData' step of the ASM, since it involves extracting the data sent by the server to check its integrity. Furthermore, moving checkCrc32 to the 'validate' step would cause the 'extractError' step to override the helpful error code of CRC32CheckFailed and would require us to override the 'extractError' listener that contains needed logic for errors other than CRC32 errors. So the PR fixes the issue by overriding the response data with null while keeping checkCrc32 in the 'extractData' step.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.