[Service Bus] ServiceBusErrorCode missing InvalidOperation and Argument related errors

[ramya-rao-a]

We added ServiceBusErrorCode to loosely mirror the ServiceBusFailureReason in the .Net SDK

Apart from the failure reason for the ServiceBusException, the .Net SDK provides its users other built in exception classes so that they can differentiate the different AMQP error conditions. In the absence of such different classes in the JS SDK, all such scenarios get lumped into GeneralError. This issue is to either consider adding error codes for such error scenarios or to consider using the built in classes in JS like RangeError and TypeError.

• amqp:not-implemented
• amqp:not-allowed
• argument-error
• argument-out-of-range

[chradek]

Initial thoughts:

• amqp:not-allowed

The peer tried to use a frame in a manner that is inconsistent with the semantics defined in the specification.

I think GeneralError is appropriate here. I'm not sure there's action a customer could take to rectify the above issue beyond what they may do for any general error (which I suspect is create a new connection or process.)

• amqp:not-implemented

The peer tried to use functionality that is not implemented in its partner.

I'm not sure what an example of this would be for Service Bus, but this also seems like the sort of error you'd see while developing an application due to a misconfiguration or possibly using a method incorrectly, and not something you'd write defensive code against. With that in mind I think GeneralError would be fine here too.

argument-error and argument-out-of-range are interesting because those look like validation errors. I don't think we currently convert and server-side validation errors to TypeError or RangeError, but maybe that's just too difficult to do reliably with the HTTP-based services. I think I'd prefer to set the error.code and then write code that checks if it matches something like ValidationError rather than make the user do an err.name check and err.code check depending on the error that comes from the service, though I don't feel too strongly about that.

@bterlson @xirzec I'd like your opinions on if we should convert server-side validation errors into native JS errors (TypeError/RangeError). I have typically thought of those as reserved for client-side validation errors but I'd like other opinions on this subject

[ramya-rao-a]

I agree with your recommendation of using GeneralError for the amqp:not-implemented case, but am not convinced for amqp:not-allowed

amqp:not-allowed
The peer tried to use a frame in a manner that is inconsistent with the semantics defined in the specification.
I think GeneralError is appropriate here. I'm not sure there's action a customer could take to rectify the above issue beyond what they may do for any general error (which I suspect is create a new connection or process.)

Well, the reaction to this error condition would be to change the code and not create a new connection or process because it would not change the set of user defined things that resulted in the frame being inconsistent with the semantics. One example is trying to send a message without sessionId to a session enabled queue.

[chradek]

Well, the reaction to this error condition would be to change the code

Sorry, I didn't word my point well. What I meant was that I didn't think a customer would realistically write defensive code for a not-allowed error. This is the sort of error I'd expect to see while developing an application, and as soon as it was encountered, I'd change my code based on the error message. That's why I was thinking a GeneralError might suffice if we don't expect customers to be able to write defensive code that executes at runtime to handle this.

[ramya-rao-a]

I didn't think a customer would realistically write defensive code for a not-allowed error. This is the sort of error I'd expect to see while developing an application, and as soon as it was encountered, I'd change my code based on the error message

That I agree with.

For errors that fall under the GeneralError case, can we consider prefixing the error message with the MessagingError.code? This way, we don't need to have a dedicated reason as we dont believe defensive code needs to be written for such cases, at the same time, we preserve some mapping to the original error condition.

[bterlson]

@bterlson @xirzec I'd like your opinions on if we should convert server-side validation errors into native JS errors (TypeError/RangeError). I have typically thought of those as reserved for client-side validation errors but I'd like other opinions on this subject

I wouldn't use these errors for anything involving service interactions, but I admit I don't have a good grasp on when these errors are thrown in practice.

I like the direction of this thread re: amqp: errors - using GeneralError with a good prefix so users can fix their code in development (and/or file a bug on us if it's our fault 😁) seems good.

[chradek]

Yup, I also like the idea of prefixing the error message with the MessagingError.code.

With regards to the argument-error and argument-out-of-range error, we got confirmation that we shouldn't be able to trigger these from the service unless there was something wrong with the SDK itself. With that in mind, I think we treat these the same as the other 2 types and just prefix the error message with the messaging error code.