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

Fix NPE where data.Error is undefined from a 400 response #3587

Closed
wants to merge 1 commit into from

Conversation

mjgp2
Copy link

@mjgp2 mjgp2 commented May 3, 2022

Description

Sometimes (seemingly on a 400 response) the Error is undefined, and data.Error.Code throws a NPE.

Testing

Existing tests should suffice.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mjgp2 mjgp2 requested a review from a team as a code owner May 3, 2022 17:38
@kuhe
Copy link
Contributor

kuhe commented May 3, 2022

Thanks for your PR. I believe this needs a corresponding change in the 🪀 🐶 code that generates the code, as seen in what #3085 is getting at.

There is no clear indication at the moment that you are editing generated code, but there is a PR open to change that: smithy-lang/smithy-typescript#538.

@AllanZhengYP
Copy link
Contributor

Hi @mjgp2 Can you also share the content of data when error code cannot be parsed? It'll be helpful to use to roll out a correct fix.

@eliran
Copy link

eliran commented Jul 14, 2022

We are hitting the same and similar issues. The proposed fix is only partial.

This is a sample of SQS 400 error (for a DeleteMessage with an invalid receipt):

Errors: {
    Error: {
      Code: 'ReceiptHandleIsInvalid',
      Message: 'The input receipt handle is invalid.'
    }
  },
  RequestId: 'PFS6BZ1KB4DNYC9BQ0N6MWQHX0XKVKTSQZRJ9WXSK6WC9AK0CPOS'

Error is wrapped inside Errors which means code isn't found and we get back a type error rather than a sensible actionable error.

Some 400s return correctly like this one (for SendMessage with large message):

{
  xmlns: 'http://queue.amazonaws.com/doc/2012-11-05/',
  Error: {
    Type: 'Sender',
    Code: 'InvalidParameterValue',
    Message: 'One or more parameters are invalid. Reason: Message must be shorter than 262144 bytes.',
    Detail: ''
  },
  RequestId: 'OLBR5M8PNYFNXHL20G90AINDLBABVK8GNINGS03SLDEIJYG3G42U'
}

I have also seen errors return without Error property at all but I don't have a current example.

This does cause us some issues with migrating from v2 to v3 which handled these errors properly.

@Tetramputechture
Copy link

Tetramputechture commented Jan 9, 2023

@mjgp2 did you have any updates on this? i am running into this and its obfuscating a small percentage of our API errors, preventing us from knowing what's actually going on. specifically, around 1/100000 calls to SNS PublishMessage when publishing to a push notification (APNS / GCM) endpoint are failing with this NPE. Specifically

Failed to execute PublishMessage command: Cannot read properties of undefined (reading 'Code')

@kuhe
Copy link
Contributor

kuhe commented Jan 31, 2023

This was replicated in #4367

@kuhe kuhe closed this Jan 31, 2023
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants