[Service Bus] Better errors for settling deferred msg in ReceiveAndDelete mode#10396
Conversation
ghost
added
the
ramya-rao-a
force-pushed
the
deferred-delete-mode
branch
from
a19cacc to
fe6f7d7
Compare
ramya-rao-a
requested review from
HarshaNalluru,
chradek and
richardpark-msft
as code owners
| // we have to force this cast - the type system doesn't allow this if you've chosen receiveAndDelete | ||
| // as your lock mode. | ||
|
|
||
| // have to cast it - the type system doesn't allow us to call into this method otherwise. |
There was a problem hiding this comment.
Repetitive?
| // we have to force this cast - the type system doesn't allow this if you've chosen receiveAndDelete | |
| // as your lock mode. | |
| // have to cast it - the type system doesn't allow us to call into this method otherwise. | |
| // we have to force this cast - the type system doesn't allow us to call into this method otherwise. |
There was a problem hiding this comment.
no idea, I copied the entire test set from the ones we have for "normal" messages in this file :)
There was a problem hiding this comment.
Yeah, the problem is that the Receiver isn't being typed more specifically.
We should fix this at some point to allow the receiver to be scoped properly but I think (grossly) this is the best we can do for now.
| receiveMode: ReceiveMode | ||
| ) { | ||
| Object.assign(this, fromAmqpMessage(msg, delivery, shouldReorderLockToken)); | ||
| if (receiveMode === ReceiveMode.receiveAndDelete) { |
There was a problem hiding this comment.
Let's just pop a comment in here saying this is a workaround (because eventually we can get rid of this, right?)
richardpark-msft
left a comment
richardpark-msft
left a comment
There was a problem hiding this comment.
Looks fine to me, just add a comment so we know why we unset the lock token.
| should.equal(errorWasThrown, true, "Error thrown flag must be true"); | ||
| } | ||
|
|
||
| it(noSessionTestClientType + ": complete() throws error", async function(): Promise<void> { |
There was a problem hiding this comment.
would it be simpler to just do the [].forEach(() => {}) trick instead?
Like this
3 participants
As reported in #10395, we have inconsistent errors being returned to the user when settling a deferred message in Receive And Delete mode.
There are 2 different root causes here
The solution being proposed in this PR is
lockTokenon the message toundefinedif the message was received in ReceiveAndDelete mode.lockTokenis then taken to be the indicator of the ReceiveAndDelete mode when settling, and a consistent error is thrown