aws / aws-sdk

Landing page for the AWS SDKs on GitHub
https://aws.amazon.com/tools/
Other
68 stars 12 forks source link

@aws-sdk/client-lightsail throws the base exception instead of a specific exception #665

Open omid-sadeghi opened 7 months ago

omid-sadeghi commented 7 months ago

Checkboxes for prior research

Describe the bug

In lightsail-client, when GetStaticIpCommand and GetInstanceCommand don't find the resource, they throw LightsailServiceException but according to the docs for GetStaticIpCommand and GetInstanceCommand, they should throw NotFoundException and the current behaviour makes instanceof useless for checking type of the exception at runtime because LightsailServiceException is the base exception. Unfortunately, I must resort to checking by the the name property.

SDK version number

@aws-sdk/client-lightsail@3.462.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.9.0

Reproduction Steps

import { LightsailClient,  GetInstanceCommand,  GetStaticIpCommand, NotFoundException, LightsailServiceException} from "@aws-sdk/client-lightsail";

const client = new LightsailClient({});

try{
    await client.send(new GetInstanceCommand({instanceName: "non-existent instance"}));
}
catch(e){
    if(e instanceof NotFoundException)
        console.log("NotFoundException")
    else if(e instanceof LightsailServiceException)
        console.log("LightsailServiceException");
}

try{
    await client.send(new GetStaticIpCommand({staticIpName: "non-existent static ip"}));
}
catch(e){
    if(e instanceof NotFoundException)
        console.log("NotFoundException")
    else if(e instanceof LightsailServiceException)
        console.log("LightsailServiceException");
}

Observed Behavior

The code outputs: LightsailServiceException LightsailServiceException

Expected Behavior

I expect to see: NotFoundException NotFoundException

Possible Solution

No response

Additional Information/Context

No response

RanVaknin commented 7 months ago

Hi @omid-sadeghi ,

Thanks for reaching out. The error you are seeing is a modeled error coming from the service. The SDK team cannot change this service side behavior.

and the current behaviour makes instanceof useless for checking type of the exception at runtime because LightsailServiceException is the base exception. Unfortunately, I must resort to checking by the the name property.

I'm not sure I understand how LightsailServiceException being the base exception prevents you from using instanceof as it is also a modeled exception.

Additionally, after probing the API and logging the actual error message alongside with the error code, I can see the Lightsail uses LightsailServiceException as kind of a catch-all exception, but they make up for it by providing very explicit error messages. So for example using your input "non-existent instance" we are going to run into a service side validation error:

LightsailServiceException InvalidResourceName: non-existent%20instance is not a valid resource name. Resource names must be at least 2 characters long and contain only alphanumerics, -, _, and .

Changing the resource name to fit the validation rules to soemthing like: "nonExistentInstance_123", will result in the same exception but a different error message:

LightsailServiceException DoesNotExist: The Instance does not exist: nonExistentInstance_123

Lastly, while this does seem like a documentation discrepancy where the service documents one thing, but returns another, it's possible that we are overlooking some combination of inputs where that exception is actually thrown. Unfortunately I do not have access to the server code to check for the error handling logic to give you a conclusive answer.

If there is anything else I can do to help, pleaes let me know. All the best, Ran~

github-actions[bot] commented 6 months ago

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.

omid-sadeghi commented 6 months ago

Hi @RanVaknin

So you say the issue is related to the service and you cannot do anything. Where can I possibly report the issue to so they can consider this?

You said:

I'm not sure I understand how LightsailServiceException being the base exception prevents you from using instanceof

I specifically point to a blog post in aws which says instead of using the name property of an exception, you can use instanceof to know the type of exception at runtime.

Because every exception is LightsailServiceException so instanceof cannot help in determing the type of error at runtime.

RanVaknin commented 6 months ago

Hi @omid-sadeghi ,

So you say the issue is related to the service and you cannot do anything. Where can I possibly report the issue to so they can consider this?

You can use the AWS console to open a support ticket and ask for it to be routed to the Lightsail service team.

Because every exception is LightsailServiceException so instanceof cannot help in determing the type of error at runtime.

Ah yes, I agree with this sentiment, the error returned is not very useful, but you can still use instanceof on LightsailServiceException.

I'm going to transfer this to the cross SDK repository since this is not an SDK specific problem. Feel free to mention my internal AWS alias rvaknin when you open the ticket, I'll be able to give the Lightsail team some more info if necessary.

Thank you for the clarification. Keep us posted. Ran~

omid-sadeghi commented 6 months ago

Thank you @RanVaknin

Unfortunately In my AWS console I have Basic Support plan which excludes technical support and I cannot open a ticket for this case.

By the way, I used workarounds to do my job but if this exception thing is corrected, we can write a better code. I hope this problem is solved.