* [ServiceBus] add delay between infinite retry cycles
Errors that are not retryable will be re-thrown out of the normal retry() call.
However, we don't have any delay before restarting the retry cycle.
This PR adds a delay before continuing the infinite retry cycles.
* Add a test and update changelog
also fixed the issue where we don't use `maxRetryDelayInMs` in Fixed retry
mode.
* Address CR feedback
* Update sdk/servicebus/service-bus/CHANGELOG.md
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
* Revert max delay limit in Fixed retry mode
as the setting only applies to Exponential retry mode.
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
when subscribing to a valid topic but an invalid or disabled subscription. Issue https://github.com/Azure/azure-sdk-for-js/issues/18798
The problem is that in these cases, the service accepts our client's cbs claim negotiation, and the receiver link is
created. The [client receiver enters](14099039a8/sdk/servicebus/service-bus/src/core/streamingReceiver.ts (L539))
the `retryForeverFn` to call `_initAndAddCreditOperation()`, where the following
```ts
await this._messageHandlers().postInitialize();
this._receiverHelper.addCredit(this.maxConcurrentCalls);
```
will return successfully, despite that the fired-and-forgot `addCredit()` call would later leads to
`MessagingEntityDisabled` or `MessagingEntityNotFound` errors in the underlying link. Since there's no errors thrown in
our retry-forever loop, the `onErrors()` [callback](14099039a8/sdk/servicebus/service-bus/src/core/streamingReceiver.ts (L541))
is not invoked. It is one place where the user error handler is called.
Because of the error, the link is detatched. We have code to re-establish the link when errors happen, so we call
`_subscribeImpl()` where the `retryForeverFn()` is called again. This goes on in an endless loop.
We used to invoke user error handler and would not attempt to re-establish connections when errors are considered
non-retryable. In PR #11973 we removed the classification of errors that `subscribe()` used and instead continues to
retry infinitely. We also removed the code to invoke user error handler. This PR adds code to invoke the user error
handler in `_onAmqpError()` so users have a chance to stop the infinite loop.
There's another problem. When users call `close()` on the subscription in their error handler,
`this._receiverHelper.suspend()` is called to suspend the receiver. However, when re-connecting we call
`this._receiverHelper.resume()` again in `_subscribeImpl()`. This essentially reset the receiver to be active and we
will not abort the attempt to initialize connection
14099039a8/sdk/servicebus/service-bus/src/core/streamingReceiver.ts (L574-L579)
To fix it, this PR moves the `resume()` call out. It is supposed to only called to enable receiver before
subscribing. It should not be called when we try to re-connect.
* Update ref doc on the receiver subscribe error handler
* Apply suggestions from code review
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
* Bring back processError() call in _onSessionError() callback
* Clarify that we retry on all errors when streaming
and link to ref doc on service bus errors.
* Bring back comments for resume() call
Also fix linting issue. The linter insists "@" in ts-doc should be escaped.
The url still works after adding "\"
* Re-remove comments
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
NodeJS would issue warning if the number of disconnected listeners on an event
emitter exceeds 10. When many sessions on a connection are closed but the
removal of the listener hasn't caught up, we will see this warning because the
default limit of 10 in NodeJS is too low. The disconnected listeners DO get
removed eventually in our code.
We already do this for senders created in Service Bus. This PR increase the
limit to 1000 for senders and receivers created off
`ConnectionContextBase.connection` so that the limites apply to both Service Bus
and Event Hubs.
* EH and SB no longer need to set max listener limit.
For Issue https://github.com/Azure/azure-sdk-for-js/issues/12161
**Packages impacted by this PR:**
- `@azure-tests/perf-service-bus-track-1`
**Issues associated with this PR:**
- #18982
**Describe the problem that is addressed by this PR:**
Add a test for receiveBatch using the track 1 service bus SDK in line with what we already have for the track 2 SDK
**Provide a list of related PRs** _(if any)_
- #18811
**Checklists**
- [x] Added impacted package name to the issue description
**Packages impacted by this PR:**
@azure/event-grid, @azure/service-bus
**Describe the problem that is addressed by this PR:**
In order to conform to the OTel spec and allow for easier aggregations we
updated tracing policy to name the span `HTTP <method>` instead of `<path>`.
Unfortunately we missed a few tests, so this updates those tests to reflect the
core changes.
While editing these tests, I realized I never added CHANGELOG entries for the original
change. This PR addresses that as well.
**Provide a list of related PRs**_(if any)_
#19838Resolves#19887Resolves#19885
* base code
* 'Retry-After',
'0',
manual replacements
* setDefaultRetryAfterIntervalInNockFixture
* setDefaultRetryAfterIntervalInNockFixture
* changelog
* changelog
* docs
* docs
* 19501
* Update sdk/test-utils/recorder/test/node/utils.spec.ts
* Format
* Update recordings to have retry-after value of 0
and remove some 202 responses
Co-authored-by: Jeremy Meng <yumeng@microsoft.com>
* [ServiceBus][Test] Fix Min-Max test build failure
With `@types/long` min version of `4.0.0` we got compilation error
```
Namespace 'Long' has no exported member 'Long'.
```
version `4.0.1` fixes this.
* Update version for event hubs too
for non-existing queue.
When this test is running on China Cloud on a Mac build agent, it gets the
expected error except that it takes a little more than 3 seconds for the error
to come back.
This PR increases the timeout to 5 seconds.
* [ServiceBus][Test] fix unstable tests that verify message state
The test messages we create have `scheduledEnqueueTimeUtc` set. Most of the time
messages received from the service have "active" state, but occasionally have
"scheduled" state due to the set property, thus failing the assertion.
This PR updates the check to check for either "active" or "schedule". In
addition, a test is added for "active" state where the `scheduledEnqueueTimeUtc`
property is removed before sending the message.
Also shown in the failing pipeline that when the test failed, the message wasn't
removed because `receiver.completeMessage()` is not called. The lingering
message caused the next test to fail. While updating the tests I also moved the
cleanup step before assertions so the message should always be removed.
* Undo changes to pipeline
* Address CR feedback
* Disable linting rule
* Remove an unstable assertion on message state.
Resolves#17075
This PR updates the version of ws from v7 to v8
The breaking changes in the changelog do not affect the way we use it. See https://github.com/websockets/ws/releases/tag/8.0.0
Chose to go with `^8.2.0` to match the version being used by `@azure/web-pubsub`
As discussed in #17076, we no longer have the need for the `docs` script in each of our packages. This PR removes this script and the related dev dependency on typedoc
This PR makes the following updates regarding the `nyc` dependency
- Update to v15 from v14 across all packages
- Updates to `@azure/monitor-opentelemetry-exporter` as it failed to run the tests with the updated nyc.
- Update the test scripts to use the js files in the dist-esm folder like all other packages instead of using the ts-node plugin.
- Update one of the tests for `@azure/monitor-opentelemetry-exporter` to use the right path for package.json file now that the tests are being run from the dist-esm folder.
Random set of live tests were triggered from this PR to ensure that nyc works as expected.
The failure for data-tables is an unrelated service side issue
Resolves#19232
## What
- Patch service-bus and event-hubs' rollup config with custom plugins that are required for test runs
- Replace `assert` with `chai`
## Why
We tried to use the shared rollup config across packages but unfortunately service-bus event-hubs are and will likely remain a special case due to the various dependencies it pulls in. We identified a few fixable issues but there are some
that will not be easily resolved.
So, working with Will we decided a reasonable compromise is to use the shared config but patch the plugins so that they
match what they were before the recent changes.
In passing, I am also removing any imports of the `assert` module as that has been removed and the tests are failing. So
this should fix that as well.
* [ServiceBus] Add optional boolean `skipParsingBodyAsJson` option
to `ReceiveMessagesOptions`, `SubscribeOptions`, and
`ServiceBusSessionReceiverOptions`. This allows users to control whether the SDK
should skip parsing message body as Json object. By default the SDK will attempt
parsing message body as Json object.
While updating code, I also fixed sample unit test which is using out-dated
paths thus not finding any sample code files, and removed two `await` that are
redundant.
Resolves#18630
* Add optional `skipParsingBodyAsJson` boolean property
to `ServiceBusReceiverOptions` and remove those on `ReceiveMessagesOptions` and
`SubscribeOptions`
* Remove commented code that was trying to decode again
but the message body is already decoded, and _rawAmqpMessage is assigned in
`fromRheaMessage()`
* Remove double decoding
message.body has been decoded already above by `fromRheaMessage()` call.
* Update CHANGELOG and package version
* Add comments
* Update sdk/servicebus/service-bus/CHANGELOG.md
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
## What
- Fix incorrect logic when suppressing chai's circular dependency warnings
- Move to the common dev-tool configuration where possible
## Why
This is a longstanding issue that we have, where an incorrect logic was copy-pasted to other places. I figured while cleaning this up that any package I touch can just convert over to the shared dev-tool configuration. Where I was unable
to do that, I just fixed this bug to avoid too many changes in one PR.
Fixes#14292Resolves#17818Resolves#17816Resolves#17815Resolves#17814Resolves#17813Resolves#17810
It can be one of the three `ServiceBusMessageState` values: `"active"`, `"deferred"`, or `"scheduled"`.
Several tests are updated to verify the message state property.
* [ServiceBus] Enable linting
* Fixing most linting errors in tests
* Fix more linting errors in src
* Disable errors
- for namespace
- parameter of `any`
* make key const and value mutable
* Add `.catch()` block for promise
* address CR feedback
* format
* Delete test input of undefined ruleOptions for createRule() test
* Use different overloads of `crateRule()`
* Update to the new rule name
* in browser, the error caught doesn't have message property!
AssertionError: expecting valid error.message but error is [object CloseEvent]: expected undefined to be truthy
Related to #14581
In https://github.com/Azure/azure-sdk-for-js/pull/18463, we updated all samples to use GA of Identity v2
In https://github.com/Azure/azure-sdk-for-js/pull/18470, we updated devDependencies of Identity v2 beta to use Identity v2 GA
This PR removes the dependency overrides put in place for the reason (am guessing) that we wanted tests to use the beta v2 Identity, but not samples
Now that the samples are using v2, we no longer need the dependency overrides
Reported in #18033.
Basically did a bulk find+replace everywhere. Things _seem_ to be working OK, but wouldn't be surprised if there's something somewhere I've missed, or somewhere where I've replaced something I shouldn't.
I've split the rename of PerfStress -> Perf and runAsync -> run into two commits for ease of review :)
* ServiceBusAtomAPIVersion
* older API Version is broken
* tests
* rename
* changelog
* getRandomServiceVersion
* remove .only
* address Jeremy's feedback
* serviceVersion comments
* docs
* test with both the api versions
* Donot expose ServiceBusAtomAPIVersion
* test
* doc update
* Fix retry handling for Session Not Found errors
* Update sdk/cosmosdb/cosmos/test/internal/session.spec.ts
Co-authored-by: Zachary Foster <zfoster@users.noreply.github.com>
* Update CODEOWNERS for Application Insights (#17055)
* MaxMessageSizeInKilobytes api
* sample,test.env
* API Report
* add tests
* finish the tests
* remove console.log
* arm template
* put .only to test in CI with limited tests
* minor test update
* remove .only, add changelog
* Update sdk/servicebus/service-bus/src/util/constants.ts
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
* wrap text - changelog
* maxMessageSizeInKilobytes: 256 by default for Standard namespaces
* Revert "maxMessageSizeInKilobytes: 256 by default for Standard namespaces"
This reverts commit 4f425634b6.
* exclude "maxMessageSizeInKilobytes"
* (Configurable only for Premium Tier Service Bus namespace.)
* Version 7.3.1-beta.1
* update version at more places
* lock file
* Update sdk/servicebus/service-bus/CHANGELOG.md
Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>
* lock file
* non optional in EntityProperties
* api report
Co-authored-by: Steve Faulkner <southpolesteve@gmail.com>
Co-authored-by: Zachary Foster <zfoster@users.noreply.github.com>
Co-authored-by: omziv <80668253+omziv@users.noreply.github.com>
Co-authored-by: chradek <51000525+chradek@users.noreply.github.com>
Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>
## What
- Update API Extractor to the latest version (currently 7.18.11)
- Regenerate all API reviews by building all the packages
## Why
This is something we keep bumping into. First, we needed to upgrade API Extractor to allow us to re-export directly from
`@opentelemetry/api`. Then, it looks like we needed this to upgrade TypeScript to 4.4.
We are way behind on this version, and it's time to upgrade.
## Callouts
How noisy is this?! Here's what's happening - somewhere around 7.9 I think API Extractor improved the way it detects name
collisions with predefined globals. Things like KeyType, Response, etc.
If there's a clash, the generated API markdown file will rename <Item> to <Item_2> to make the name collision explicit.
Talking to folks on the team, and the poor souls that will be doing API Review approvals, we agreed that doing the upgrade
in one fell swoop is the way to go.
Resolves#9410
* Add PoliCheck tool to aggregate report pipeline
* Exclude recordings from policheck run
* Use upper case for folder exclusion
* Fix up policheck issues
* Move CredScan into the Compliance stage
* Fixup policheck issues
Resolves#17089
`@azure/abort-controller` doesn't use `delay` in any tests, so easy removal.
`@azure/service-bus` used `delay` for perf tests, but only as a promisified setTimeout. Removed dependency and replaced with a simple delay function.
* Remove leftover recordings whose tests had been deleted
* Move test secrets into file so we can suppress the file, instead of suppressing using the unstable hashes
* Update suppression list
* move to the new structure
* minor edits
* sample.env and readmes
* more changes
* track 2 service-bus looks fine
* track 1 also seems fine
* more fixes
* formatting
* Update sdk/eventhub/perf-tests/event-hubs-track-1/README.md
Co-authored-by: Daniel Rodríguez <sadasant@users.noreply.github.com>
* Update sdk/eventhub/perf-tests/event-hubs-track-1/README.md
* Apply suggestions from code review
* lock file
* lock file from master
* lock file
* all minor feedback
* more formatting
* remove prebuild
* more care for readmes
Co-authored-by: Daniel Rodríguez <sadasant@users.noreply.github.com>
## What
- Removes spanOptions from OperationTracingOptions
- Prepares client package for compatibility with both existing and future core-tracing code by adding runtime casts / checks where spanOptions are concerned
- Adds `az.namespace` as an attribute on the current context, ensuring that it is available for tracing policies
- Adds spanOptions as a parameter to `createSpan` allowing customization of a newly created span
- Prepare tracingPolicies for backwards and forwards compatibility at runtime by using spanOptions as well as spanContext to determine the value of az.namespace to set on spans.
**Does NOT upgrade client libraries yet and their API will NOT change until we upgrade**
## Why
spanContext originally existed to be able to shepherd attributes around in order to add them to any child spans. Specifically, we need to add the az.namespace to the outgoing request span. Now that `tracingContext` is available as both a bag of values as well as a container for parenting info, we no longer need spanOptions.
Actually, spanOptions grew to add enums and other elements that caused us pain in the past. See #15285 as an example.
Finally, providing spanOptions as a publicly available option bag means that we are providing ways for consumers to customize our own spans. We don't need that, as we own the spans we create.
But what we still need to provide is a way for our own libraries to customize the spans they produce. By moving spanOptions off of OperationTracingOptions and into a separate parameter in createSpan we can support this.
## Callouts
I tested compatibility here in multiple ways:
1. I changed core-tracing's version to preview.13 so that all libraries will run tests against the new package
2. I then changed it _back_ to 14, so that all libraries will run tests against the old pacakge
3. I packed preview.13 with new bits and installed published packages for a few libraries and manually tested a few scenarios
Resolves#15285Resolves#16726
* [Identity] Incrementing the package version
* setting samples to the last released version of Identity
* upgraded non-samples to 2.0.0-beta.6
* found a fix for the CI issue
* Re-organize eh stress files for stress test layout
* Add service bus stress test helm chart
* First pass generating stress job for each scenario
* Deep copy global template scope in stress test range loop
* Updates and deployment files for service bus stress
* Use base arm template
* fix helm template and docker build
* Final fixes to get service bus stress tests working
* [stress test] Update to use built-in scenario range addon
* [stress test] Minor fixes
* Update stress test program locations
* Run prettier over service bus stress test files
This PR adds missing changelog entries for the times we
- updated tracing dependencies to use the GA version of OpenTelemetry
- updated to target ES2017
## What
- Remove `setTracer`
- Remove `NoOpTracer` and `NoOpSpan`
- Use Otel's APIs where possible (like when creating a no-op)
- Respect AZURE_TRACING_DISABLED environment variable
- Include test support for tracing utilizing OTel's trace API
- Avoid injecting invalid `tracerparent` header fields
## Why
- `setTracer` was added before OTel had their own implementation of a global tracer provider. For consistency with other libraries we should use the global tracer that was registered. It also leaves us out of the business of maintaining caches, global tracers, and other annoying things.
- These are no longer needed, since OTel has a recommended way to wrap a span in a NoOp. And, if no tracer provider was registered, a non-recording tracer (NoOp) will be created. All managed by OTel
- Finally, AZURE_TRACING_DISABLED is one of the env vars our guidelines say we should support
Resolves#16088Resolves#15730Resolves#10205
## What
- Move `TestTracer`, `TestSpan`, and `SpanGraph` from @azure/core-tracing to @azure/test-utils
## Why
1. Having a folder called src/**test**/ does not play nicely with defaults generated by tools such as Yarn (specifically Yarn autoclean)
2. These are test support interfaces, and shouldn't be part of our public API anyway - now that we have @azure/test-utils it's the more appropriate location for them
Resolves#16265Resolves#16201
Related #13367
We have an intermittent issue where we close a session and it releases, but there is a delay (Documented as part of this issue here: [link](https://github.com/Azure/azure-sdk-for-js/issues/14441). We've seen it in our normal live tests (not every time, but often enough to be noted).
I've done two things to make this "better":
- Mitigated the issue so we can still get a decent test signal by making the tests no longer use the same session ID. They didn't actually need to send or receive messages for the test itself (it's just validating that we're adding the right amount of credits) so I simplified the tests.
- Put in some wrapper code to make it simpler to reproduce the issue in the future. It will basically run these session based tests 1000x, which still doesn't _always_ reproduce the issue but can.
As a bonus, there was a `peekMessages()` test that expected that a message that has been sent will immediately be available, which isn't always true (there can still be a delay between "message accepted" and "message receivable"). I've fixed this test by doing a `receiveMessages()` first (which will wait until messages are available), abandoning the message and then doing a peek, which should work more reliably.
Fixes https://github.com/Azure/azure-sdk-for-js/issues/15666
It also turns out the samples depended on the `next` tag for service-bus, which is no longer published on NPM, so this update fixes that issue by depending on the `latest` tag.
* displayed links as a list rather than a single line
* appconfiguration\Readme: displayed links as a list rather than a single line
* confidential-ledger-rest/README.md: display links as list
* container-registry/README.md: display links as list
* cosmos/README.md: display links as list
* iot-device-update/README.md: display links as list
* eventgrid/README.md: display links as list
* event-hubs/README.md: display links as list
* eventhubs-checkpointstore-blob/README.md: display links as list
* formrecognizer/README.md: display links as list
* identity/README.md: display links as list
* iot-modelsrepository/README.md: display links as list
* keyvault-admin/README.md: display links as list
* keyvault-certificates/README.md: display links as list
* keyvault-keys/README.md: display links as list
* keyvault-secrets/README.md: display links as list
* ai-metrics-advisor/README.md: display links as list
* mixedreality-authentication/README.md: display links as list
* monitor-query/README.md: display links as list
* purview-catalog-rest/README.md: display links as list
* purview-scanning-rest/README.md: display links as list
* quantum-jobs/README.md: display links as list
* search-documents/README.md: display links as list
* schema-registry/README.md: display links as list
* schema-registry-avro/README.md: display links as list
* service-bus/README.md: display links as list
* storage/storage-blob/README.md: display links as list
* storage-blob-changefeed/README.md: display links as list
* storage-file-datalake/README.md: display links as list
* storage-file-share/README.md: display links as list
* storage-queue/README.md: display links as list
* data-tables/README.md: display links as list
* ai-text-analytics/README.md: display links as list
* video-analyzer-edge/README.md: display links as list
* web-pubsub/README.md: display links as list
* web-pubsub-express/README.md: display links as list
* core-lro/README.md: display links as list
* changed from the word master to main
* changed the word master to main
* another update to the final reandme to change the word master to main
* Update README.md
fixed a type in the link
* Update sdk/anomalydetector/ai-anomaly-detector/README.md
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
Fixing an issue where we could lose messages or provoke an alarming message from rhea (`Received transfer when credit was 0`)
The message loss issue is related to how we trigger 'drain' using 'addCredit(1)'. Our 'receiver.drain; receiver.addCredit(1)' pattern actually does add a credit, which shows up in the flow frame that gets sent for our drain. This has led to occasionally receiving more messages than we intended.
The second part of this was that we were masking this error because we had code that specifically threw out messages if more arrived than were requested. If the message was being auto-renewed it's possible for the message to appear to be missing, and if we were in receiveAndDelete the message is effectively lost at that point. That code is now removed (we defer to just allowing the extrra message, should a bug arise that causes that) and we log an error indicating it did happen.
The rhea error message appeared to be triggered by our accidentally allowing multiple overlapping 'drain's to occur (finalAction did not check to see if we were _already_ draining and would allow it to happen multiple times). Removing the concurrent drains fixed this issue but I didn't fully investigate why.
Fixes#15606, #15115
## What
- Update core-http to 2.0.0
- Update core-lro to 2.0.0
- Update packages to use latest version
## Why
This will support our last breaking change for core-tracing, and allow everyone to be on the same minimum version. This will also allow us to target ES2017 across the board.
Today we cache any opened links in the connectionContext. These links should be removed when the link itself is closed but, due to a mismatch in the values, we weren't.
I've fixed this by just making an abstract method in LinkEntity (the base for all the link types) and just having each link properly remove itself from the cache.
Fixes#15890
Making changes to simplify a flaky test (and hopefully make it more reliable).
The main issue with the 'handle interrupted detach' method was that it relied on too many moving parts to work reliably. We could just eternally loop like we'd expect customers to do, but in the end we have a very simple test we're trying to perform - we want to receive, and while we're in the process of draining, cause a detach and have it early exit and reject/resolve immediately rather than waiting for the timeout.
I reworked the test to make that simpler by just removing the unneeded connection.idle() and just calling directly into the onDetached method. Because it happens prior to rhea even seeing that we're draining we should reliably win that race each time.
There were a couple of other things changed for this PR as well:
- The max time per test was lowered accidentally. Bringing it back what's been used as the standard time in other libraries
- Fixed a spot in receiveMessages() where, if the link had been closed, we'd falsly return an empty array instead of throwing an exception indicating the link closed. This didn't appear to be related to the bug but it's incorrect and can hide bugs so I'd rather just throw the error than eat the condition and return an empty array. It's rare, but when it does happen the empty array response isn't right either - we're probably in the middle of a connection reset/change event.
Fixes#13461
## What
- Bump @opentelemetry/api to 0.20.0 in @azure/core-tracing
- Move all packages that are on core-http to the next core-tracing version
- Remove version collision check from the tracer cache
## Why
This is part of our effort to move everyone to OTel 0.20.0 - but we have to stage it due to a transitive dependency and a
breaking change in OTel. This PR updates core-tracing to use the latest OTel, fixes any breaking changes, and moves
packages that we can move to the latest version of core-tracing.
Once core-rest-pipeline 1.1.0 is GA'd we'll be able to move the rest of the packages over to the latest core-tracing as well.
Removing the version collision came out of an offline discussion after these changes were reviewed - since it's a common
source of pain and hasn't added much benefit (tracer conflicts incompatibility was never a problem) we decided to remove
that logic and always grab a unique symbol per OT compatibility.
## Callouts
The packages that are already on core-v2 will _not_ be upgraded at this time - we'll coordinate that with the GA of core-rest-pipeline 1.1.0
As we are ending the support for IE 11, this PR remove doc and test logic that
handle IE.
- Remove commented polyfill from karma configs
- Remove IE compatibility section from Stroage README files
- Remove IE specific logic from storage tests.
- Remove Storage TODO comments for IE.
* format
* run format on core-client-rest
Updating doc comment for autoComplete, indicating that it will abandon() if an error is thrown.
The prior text made it sound like it would call complete() no matter what happened, which was confusing.
Fixes#15519
* [service-bus] pass abortSignal to link initialization and awaitableSender
* [service-bus] add changelog entry
* [core-amqp] remove AsyncLock!
* [service-bus] fix test after the great merge
* npm run format
* fix abort on send after great merge
This PR fixes#13500.
`rhea` 2.0.1 contains the fix to this specific error. We currently use `rhea` 1.x, so there's additional work in this PR to workaround the single breaking change in `rhea`, and the breaking changes in `rhea-promise`.
### rhea breaking change
`rhea` contains 1 breaking change between versions 1.x and 2.x: timestamp types are now deserialized as Date objects instead of numbers. Unfortunately since this changes the way users' data might be deserialized in their service bus messages or event hubs events, we have to convert Date objects back to numbers in our client libraries until we do a major version bump. (Shorter term we can look at using rhea's default behavior behind a flag.)
### rhea-promise breaking changes
Some of the `rhea-promise` APIs that accepted multiple optional positional arguments have been updated to take a single options bag parameter at the end of their method parameter list.
AwaitableSender was also updated so that a timeout is no provided at instantiation. Instead, it must be provided per each `send()` call.
### core-amqp v3
Since core-amqp is being updated to depend on rhea 2.x, core-amqp dependencies will also pull in rhea 2.x transitively.
To ensure that existing versions of event hubs and service bus don't break by deserializing timestamps as Date objects, core-amqp is updated to a new major version: v3.
Once #15349 is merged, we can also remove `AsyncLock` completely, so I'd like to merge that PR in before releasing the changes in this PR.
Updating samples to fix a few issues:
- Show the right way to restart your receiver when using sessions (which
don't do automatic retries like non-sessions).
- Some of the samples printed almost too little console output to indicate what
they were doing or didn't demonstrate the right techniques (asking for multiple
messages at a time, etc..). Those have been updated as well.
Fixes#14531
getMaxMessages had an edge case where the link being closed just after open would cause us to access a link that was undefined and throw a non-retryable exception.
This PR fixes that by checking that the link is defined, and throwing a retryable exception if it is not.
Fixes#15398
Finishing work to enable AMQP body type encoding
- We completely missed the schedule messages code path. Added, with tests.
- Consolidated the two files of AMQP messaging tests into the `public` branch so we also get min/max testing. Removed all references to non-published interfaces.
Re-enabling the AMQP body type encoding feature. We disabled this temporarily when we released 7.1.0 and are now re-enabling this in time for our new preview: 7.2.0-beta.1
Disable amqp body type support until we release it in an official 'beta'.
Runtime prevention:
- Prevent isAmqpAnnotatedMessage() from ever returning true (disables all AMQP body type related code paths)
- Fix error message constants so they feature-switch and don't mention AmqpAnnotatedMessage
Compile time prevention:
- Remove AmqpAnnotatedMessage from the public facing types (ServiceBusMessageBatch and ServiceBusSender)
All tests and internals are intact and tests are enabling the code paths internally so tests are still working properly.
This PR has a few changes in it, primarily to improve our robustness and our reporting:
General reliability improvements:
- Migrates to a workflow that treats subscription start as a retryable entity, rather than just link creation (which is what had previously).
- It checks and throws exceptions on much more granular conditions, particularly in addCredit
- Error checking and handling has been migrated to be in far fewer spots and to be more unconditional, which should hopefully eliminate any areas where an exception or error could occur but it never gets forwarded or seen.
SDK debugging:
- Adds a new SDK only flag (forwardInternalErrors) which will make it so areas that used to eat errors now can forward them to processError. Prior to this the errors were only logged, but that meant they could be missed. Most of these would probably be considered cosmetic by customers so this is primarly for debugging purposes within the SDK itself.
- The internal `processInitialize` handler has been split into two (still internal) handlers - preInitialize and postInitialize. preInitialize runs before init(), and postInitialize runs after init() but before addCredit. This lets us write more reliable tests. These are not exposed to customers.
Fixes#14535
Target a Kubernetes cluster, rather than ACI!
- Change the logic from being a .js file (which nobody seemed to like) to being a set of bash scripts testBuild.sh to build the conntainer, testRun.sh to actually run the containers + scripts in Kubernetes and testAll.sh to launch every scenario file as a pod in Kubernetes.
- Fixed the stress tests to clarify some boundaries and make it simpler to write a test properly (ie, don't forget to close a client, don't forget to report telemetry or flush, etc..)
There were two live tests that were basically exercising a small segment of validation code to make sure you can't try to receiveMessages() and subscribe() at the same time. These tests have been flaky in the past, and were really only testing logic so I just migrated them to unit tests instead.
One of my previous PRs merged earlier than the live test execution. Fixing some issues that were remaining:
- the ServiceBusClient in the test wasn't being closed after each test (sessionsRequiredCleanEntityTests.spec.ts is special in that it has to recreate new entities for each test). This is responsible for the current 3+ hour test executions happening in master for SB.
- Removed a stray console.log statement I left in the test.
This commit adds in a receiveMessages call to ensure that messages have arrived by the time we actually do our peekMessages() call. Without this, if there is a delay between the message being processed in the queue and it being available for receiving we'll end up in a situation where peekMessages() will return zero messages.
peekMessages goes against an expected sequence number and doesn't wait if there aren't any messages at that moment to receive. receiveMessages() always asks for the current messages and will wait for the expected number to arrive. So by adding peek _after_ receiveMessages() we should be able to get the proper sequence of operations - messages arrive, are available and now we can peek properly.
Fixes#14757
Fixes#12883
## Bug
"Invalid date" for the `expiresAtUtc` property
```
azure:service-bus:messages:verbose AmqpMessage to ServiceBusReceivedMessage: {
_amqpAnnotatedMessage: {
header: { deliveryCount: 0 },
.
.
.
expiresAtUtc: Invalid Date,
.
.
.
}
```
## Background
"ttl" on the received message will be undefined if the sent message doesn't have the "ttl" property set to a value.
In this case, the service assumes the default infinite value for expiry time.
Reference: https://docs.microsoft.com/en-us/azure/service-bus-messaging/message-expiration#entity-level-expiration
### Cause (expiresAtUtc)
In the SDK, for a received message, we calculate the expiresAtUtc using the "ttl" and the enqueued time.
- If the msg.ttl > max_duration - msg.enqueuedTime, calculate msg.expiresAtUtc with max_duration
- If the msg.ttl < max_duration - msg.enqueuedTime, calculate msg.expiresAtUtc with msg.enqueuedTime + msg.ttl
- If the msg.ttl is not defined on the received message, we return Invalid Date for the expiresAtUtc property.
## .NET SDK
TTL is defaulted to TimeSpan.MaxValue in .NET.
For ExpiresAt calculation:
- if AbsoluteExpiryTime is populated we use that.
- if TTL > maxDateTime - EnqueuedTime, use TimeSpan.MaxValue
- Else use EnqueuedTime + TTL
## Fix for JS SDK
If the msg.ttl is not defined, set it to the max value as the service says that. This allows the proper calculation of `expiresAtUtc`.
## TODO
- [x] Bug Fix
- [x] Test
- [x] Changelog
This was the source of a few live test pipeline failures - it was not resilient against network failures.
RequestResponseLink retries will come in a separate PR as that's a bit more involved.
(also, renamed receiver/shared.ts to receiver/receivercommon.ts, just to reduce file naming confusion amongst multiple shared.ts files)
Partly related to #13796
The ordering of properties in the XML requests to the Service Bus ATOM API is significant and changing it can have side effects.
In this instance, the ordering issues caused us to appear to have setup forwarding properly for a queue but forwarding was NOT actually enabled. Our previous testing missed this because this data actually round-trips properly through the API but it doesn't trigger whatever actual setting needs to happen to cause forwarding to happen.
This PR reconciles our queue, topic and subscription entities ordering against the .net layer, which acts as the de-facto authority.
Fixes#14539
`@azure/core-auth` added the `NamedKeyCredential` and `AzureNamedKeyCredential` symbols in version 1.3.0. Currently only `@azure/core-amqp` and `@azure/event-hubs` references these directly and need to be using 1.3.0, but updating in all packages that depend on it to satisfy rush.
* Upgrading to opentelemetry 1.0.0 (rc.0)
Did a few things that made this MUCH easier.
Now that everyone is using the createSpan from @azure/core-tracing we
no longer need _every_ project to reference opentelemetry/api! That has
been removed as part of this PR.
Unfortunately, the leaky nature of JS means that packages still need to
worry about opentelemetry when they build their browser bundle for
testing. To make that simpler I've added a common function to dev-tool
that everyone can call in their rollup that will give them the correct
named exports. This is hooked up for everyone at this point, so the next
time something like this happens I should be able to control it
centrally.
Now for the API breaking changes that I had to fix:
- CanonicalCode is gone and is replaced with SpanStatusCode.
SpanStatusCode has a much smaller set of codes (literally: ERROR, OK
or UNSET) so that simplified most of the way we were handling setting
a span status on error.
- There is a new field (`tracingContext`) that contains `Context`. You
now pass a context, not a span, to indicate what your "parent" is.
You'll see this where I've removed `SpanOptions.parentSpan`. Mostly
it's a simple replacement.
This PR has the below changes to ensure we use the standard error message for AbortError in Service Bus and Event Hubs
- Use the latest rhea-promise that uses the standard error message
- Update core-amqp to use the standard error message and export the same'
- Update EH and SB to use the standard error message from core-amqp
Fixes#13838
### Issue https://github.com/Azure/azure-sdk-for-js/issues/8875
### What's in the PR
"disconnect"/refresh logic added before did not cover the session scenarios.
This PR attempts to tackle the batching receiver for sessions upon "disconnect" scenarios to have a smooth-resolution/throw-errors based on the receiveMode.
Streaming receiver calls processError with SessionLockLost and closes the link
### receiveMessages - Scenarios to handle/test
- [x] throws "session lock has expired" after a disconnect
- [x] returns messages if drain is in progress (receiveAndDelete)
- [x] throws an error if drain is in progress (peekLock)
- [x] returns messages if receive in progress (receiveAndDelete)
- [x] throws an error if receive is in progress (peekLock)
### Streaming receiver
- [x] Test - calls processError and closes the link
### TODO
- [x] Cover the scenarios above
- [x] Tests
- [x] Bug fix - number of receivers - https://github.com/Azure/azure-sdk-for-js/issues/13990
- [x] Changelog
- [x] Streaming receiver - ~~beyond the scope of this PR~~ https://github.com/Azure/azure-sdk-for-js/issues/14212
- [ ] Bad log messages https://github.com/Azure/azure-sdk-for-js/issues/13989 - beyond the scope of this PR
- [ ] Stress testing plan for disconnect - https://github.com/Azure/azure-sdk-for-js/issues/13988 - beyond the scope of this PR
Fixes#8875
* [dev-tool] Experimental samples publish command
* Updated template samples structure
* First generation of template samples
* Update to ts-node 9 and use transpilation mode for speed.
* Many improvements and fixes.
- Fixed several bugs with generation.
- Added environment variable analysis.
- Refactored modules for code organization.
- Added azsdk- JSDoc tags for weighting and ignoring samples.
- Made almost all illogical situations yield errors instead of warnings.
* Rework text analytics
* Fixed a bug in the README template saying something about the Template package
* Regenerate text analytics samples
* Consistency changes to dev-tool/register
* Updated TA and Template package.json
* Fixed a couple of file name rendering bugs in the template
* Added API ref link override and regenerated Template samples
* Format
* Fix broken link
* Made typescript version reflect dev-tool ts dependency
* Revert weird change to cosmosdb package.json
* Alpha sort template deps
* Added MIN_SUPPORTED_NODE_VERSION
* Tweaked default tsconfig.
* Use version 1.0.0 instead of 0.1.0
* Pull sample generation info types into their own module.
* Added resource creation link generation.
* Regenerate template samples
* Regenerate text analytics samples
* Regenerate text analytics samples
* Regenerate template samples
* Fix bug in TA samples
I confirmed with the TypeScript team that patch releases should not introduce breaking changes. This PR uses tilde in TypeScript version we use so get the latest patch releases for v4.1.
Expanding the types for applicationProperties to allow `null` (and not `undefined`).
`null` and `undefined` both get mapped to 'null' if you round-trip through JavaScript. Sending through .net will properly map to null as well.
Fixes#14329
- Making some changes to simplify the "duplicate" models that we're exporting to mirror the opentelemetry models (addresses feedback from @xirzec and @bterlson)
- Switch core-tracing back to the `-preview` version naming style. Changing it mid-stream like we did breaks internal tooling.
* [service-bus][test] extract key values into constants
CredScan is computing hashes to identify locations of possible sensitive data.
Changes unrelated to the key values can cause the hashes to change. This PR
extract key values into constants in a separate file so the scan results remain
stable.
* Keep just one testConstents.ts under test/public/
* Update live test templates to use matrix generation
* Update sdk live tests to use matrix generation, cloud config stages
* Fix live test matrix filter parity errors
* Remove matrix filters. Opt-in most tests to samples and min/max testing
* Fix post step template parameter in monitor live tests
* Filter dependency version for live tests that don't support it
* Only publish test results for browser and node tests
When we were instrumenting spans we wouldn't pay attention to whether we were currently recording or not, which could lead to us creating a bunch of empty/null spans and trying to add them as links to the span for our batches.
Rather than do that I just check (when we instrument) if the span we created is recording and if not don't add it.
Fixes#13049
* [template] Make README Logging section consistent
We might have introduced the inconsistency for some libraries while copying
content from other languages.
* Update other README files
Contents for messaging libraries (Event Hub/Service Bus) remain unchanges
because they are different in having additional information about `rhea`
logging.
* linking to logger
Removing custom createSpan code in favor of core-tracing's createSpan functions. As part of this I also changed messageBatch.tryAdd() options related to tracing so it matched what OperationOptions.
Also, fix in core-tracing so you can pass in a custom SpanKind (we were previously overwriting it).
As part of prepping for the next release of OpenTelemetry we found some code patterns that were going to become a large maintenance burden for us, primarily around the parenting of spans. To make this easier I've removed as many duplicate implementation of createSpan and tried to centralize everything into core-tracing instead.
This won't completely remove changes needed for a newer version of OpenTelemetry but it'll eliminate one of the bigger bottlenecks.
Was looking at this test to compare the scenario during sessions as part of #8875.
Talked to @richardpark-msft and we agreed to simplify it as shown in the PR with slightly more meaningful comments.
This PR fixes about 150 linter errors and warnings in the test files of the Service Bus package.
Use `Hide whitespace changes" option to look at the real impacting changes
Related to #10781
This PR fixes over 90 linter errors around the `@returns` tag in Service Bus. Related to #10781
- Remove the type info in the `@returns` tag as this is redundant. TS tooling can figure out the typing info without needing us to have it add it here
- Remove the entire tag if it was not adding any information other than the type
* [EventGrid] Update core-auth reference
In b6040c5451 we took a dependency on
some new surface area in @azure/core-auth. That has now been released as
version 1.2.0, so we bump our minimum dependency to that version.
The other packages have been updated as well, to keep versions consistent.
* [EventGrid] Prepare for 3.0.0-beta.3 release
After @KarishmaGhiya's min-max PR and the reorganization,
We don't have any tests at the top level, hence deleting the `test/*.spec.ts` from the mocha test file inclusion.
Added in the "long running test" that I had previously written, now ported to @HarshaNalluru's much nicer stress framework. This test is intended to run for long periods of time, sending messages every second (and receiving the entire time). This should complement some of the other tests we have that test more stressful/high perf conditions by allowing us to see how our code handles running long-term.
Also, some other changes for quality of life and others:
* Make tsconfig.json more strict to catch more errors and other "risky" constructs in the stress tests.
* Fix aforementioned risky and erroneous constructs (mostly just using before initialization, assuming falsy, etc..)
* Refactored stressTestBase (not too drastically) to allow for tracking statistics generated from outside of the stressBase. This lets you take advantage of the snapshotting and reporting but still be able to write all your own logic. More refactoring possible, but left as an exercise to a future reader (probably me later).
* [dev-tool] Use //sampleConfiguration for skips
* Replace all instances of smokeTestConfiguration with sampleConfiguration
* Formatting
* Tested file matching logic
* removed a left-over import
There were some mechanical transform fixes we could for linting (beyond errors that could be autofixed). There are still _quite_ a few more remaining but this is the easy set that's more or less just rote transformations.
There are no code changes to speak of - this entire change occurs in the comments.
Part of a series of changes for #10781
### New perf folder
- The new "perf" folder uses the perf framework.
- With the perf framework, only the send test can be written.
- This PR adds the send test for track 1 and track 2.
- This would be useful in comparing track 1 and track 2, as well as for the cross-language comparisons.
- The track-2 tests depend on the current code on master and would require updates if the source code changes.
#### To execute the track 2 tests
1. Build the service-bus package `rush build -t service-bus`.
2. Navigate to `service-bus` folder `cd sdk\servicebus\service-bus\`.
3. Create a service-bus namespace and populate the .env file at `servicebus\service-bus` folder with `SERVICEBUS_CONNECTION_STRING` variables.
4. Run the tests as follows
- batch send
- `npm run perf-test:node -- BatchSendTest --warmup 2 --duration 7 --parallel 2`
#### To execture the track 1 tests
1. Navigate to `test-utils\perfstress` folder `cd sdk\test-utils\perfstress\`
2. Build the package `rush update && rush build -t test-utils-perfstress`
3. Pack the perf package `rushx pack`
4. Navigate to `service-bus\perf\track-1` folder `cd sdk\servicebus\service-bus\perf\track-1`.
5. Install the perf package `npm i ..\..\..\..\..\test-utils\perfstress\azure-test-utils-perfstress-1.0.0.tgz`
6. Run `npm install` to get `service-bus V1`.
7. Create a service-bus namespace and a queue with default options. Populate the .env file at `servicebus\service-bus` folder with `SERVICEBUS_CONNECTION_STRING` and `QUEUE_NAME` variables.
8. Run the tests as follows
- batch send
- `npm run perf-test:node -- BatchSendTest --warmup 2 --duration 7 --parallel 2`
### Existing perf tests
- There is an existing "perf" folder with the tests for send and receive for `azure-sb`, `rhea-promise`, `service-bus-v1`, and `service-bus-v7` libraries.
- These tests show the memory consumption too in the report. They do not use the perf framework. (And the perf framework isn't yet capable of handling the receive scenarios.)
- Retaining the existing tests for the above reasons.
- This PR renames the "perf" folder(with existing tests) to "perf-js-libs".
* Fixing a few problems and making telemetry better:
* The summary events now show all the individual counts as queryable fields in customDimensions
* Fixed a bug in runContainer.js and also made it easier to launch a container for any arbitrary script
* Added in an additional testName field to make it simpler to group telemetry by run
* Some changes to make our stress tests easier to run in containers:
* Make a Dockerfile to hold the files
* Make a script (runcontainer.js) which allows you to easily build, push and create an ACI to run the stress test
* Updated the tests against the GA version of Service Bus (they were still using preview.8, aka Service Bus "classic")
### Issue
https://github.com/Azure/azure-sdk-for-js/issues/13048
### Cause
Error thrown from the `actionAfterTimeout` method which is used as a callback for setTimeout cannot be caught in the try-catch at any level leading the program to crash.
### Fix
Make a promise and reject with error instead of throwing in the setTimeout callback.
### Changes in the PR
- [x] The mentioned bug fix
- [x] `... request with message_id "undefined" timed out ...` is thrown. Adds a uuid if none present
- [x] Test
- [x] Changelog
## Bug
- `receiveMessages` method returned zero messages upon a connection refresh caused by a network disconnect(simulated in the test).
- `OperationTimeout` error on message settlement after the disconnect
These two failures made the "disconnect" test occasionally fail for the batching receiver.
## Cause
`onDetached` on the batchReceivers is called 300ms after the connection refresh causing the recovered receive link to be closed.
- If the message returned from the service took close to 300ms until reached the receiver since the refresh, `onDetached` is called to close the link leading to the loss of messages.
- If the 300ms had elapsed right before the settlement, we'd see the OperationTimeout error on settlement since the receive link is closed.
Investigated here https://github.com/Azure/azure-sdk-for-js/pull/13339
## Fix
- Call `onDetached` for the batching receivers before calling the refresh connection
- And retain calling `onDetached` for the streaming receivers after the refresh connection
## Changes in the PR
- [x] Refactored "calling onDetached" part
- [x] Removed the 300ms delay since we don't see the utility
- [x] Changelog
- [x] TODO: What to do for sessions?
- [x] Needs more investigation https://github.com/Azure/azure-sdk-for-js/pull/13374#discussion_r564139864, will be handled at #8875
Investigated at https://github.com/Azure/azure-sdk-for-js/pull/13336 to figure out and fix the flakyness.
Sometimes, the service returns zero messages for the peek call and is forcing us to retry the peek operation until we get the message so the test is green.
(We may have other tests that may fail with the same, we can extend this fix to those tests as needed.)
This PR removes our dependency on the unmaintained package "karma-remap-istanbul" and replaces it with a smaller karma plugin ("karma-sourcemap-loader") that allows karma-coverage to load source maps from the disk correctly.
I tested and confirmed that the generated coverage data has the correct source TS files.
This PR adds
- a new option `sendAllMessagesBeforeReceiveStarts` which would be useful if we want to receive a ton of messages in the queue.
- tracking more info related to the messages
- other minor fixes
### User issue
User complained that the active message count that was returned in the subscription runtime properties is zero when there are three messages. #13219
Reason for the failure is that the manually written parser in the service-bus JS SDK looks for the "d2p1" xmlns-prefix while the response had "d3p1" as the xmlns-prefix.
### More Background
`MessageCountDetails` from the list runtime method has "d2p1" prefix whereas the runtime props method for a single subscription has "d3p1" prefix.
Swagger mentions only the "d2p1" prefix: 1f4095f20a/specification/servicebus/data-plane/servicebus-swagger.json (L367)
### Fix
As a fix for JS, since we have a manual parser and not depending on codegen, this PR attempts to get the xmlns prefix from the response to be able to obtain the details.
### Followup
.NET, Java, and Python SDKs do not fail in the same way as they don't depend on the prefix that is mentioned in the swagger to parse the details.
We had reverted the custom endpoint address changes in #13096 so that we could do a core-amqp release.
This PR reverts that revert and removes the 'beta' moniker from the event hubs version number.
Original PR: #12909
* [Docs] Upgrade typedoc to v0.15.2 to match that used by docs team
* standardize the docs script for cosmos, kv-admin, comm-common, and storage-internal-avro
### Problem
Send 10K messages... 10K messages are received just fine for non-sessionful queues. For sessionful queues, receiving stops after receiving 2048 messages with `subscribe` method and leaves the receiver hanging. (Same with `receiveMessages` API if requested for a large number of messages or called in a loop)
### Reason
The difference between messages from sessionful and non-sessionful queues is the "settled" flag - it is "true" for non-sessions but "false" for sessions which makes the circular buffer full. ("rhea" checks the "settled" flag to pop the deliveries from the circular buffer)
The "settled" flag is updated in rhea mainly by a couple of methods, auto_settle and auto_accept. And this "auto_accept" method can be invoked by setting the "autoaccept" option to true in the receiver options while creating the receiver in the receiveAndDelete mode, which is being set for the non-sessions case, but not for the sessions at the SB SDK level.
### Fix
Set the "autoaccept" option to true in the receiver options for the receiveAndDelete mode for sessions to be able to clear the buffer in "rhea".
### Related issues
https://github.com/Azure/azure-sdk-for-js/issues/13109https://github.com/Azure/azure-sdk-for-js/issues/11633https://github.com/Azure/azure-sdk-for-js/issues/8875
### TODO
- [x] There is common code in `messageReceiver.ts` and `messageSession.ts` that can be refactored.
- [x] Test that this PR fixes the receiveBatch scenarios too
- [x] Changelog
https://github.com/Azure/azure-sdk-for-js/pull/13105 investigates the flakyness.
The reason why the message count in the test can sometimes(5/1000 times) be zero is that the sent messages might not have been filtered into the subscription by the time we call the runtime properties method.
If we call the receiveMessages directly, the inherent delay would help in receiving the messages and should be enough for the test.
## What
Audit Track 2 libraries and address any linting errors in package.json
Fixed anything that seemed safe, and anything that seemed like a
potential breaking change was confirmed with the owners or reduced
to a warning.
## Why
Another step towards being able to run the linter in our builds.
fixes#10597
### BUG
https://github.com/Azure/azure-sdk-for-js/issues/13063
The correlation filter with the "subject" set using the `createRule` method doesn't filter the messages to the subscription. However, when we create the rule with the same correlation filter from the service bus explorer, messages get filtered as expected.
### CAUSE
The key-value pairs having undefined/null as the values can be present in the request object at any level. They get serialized as empty tags in the XML request for the ATOM management operations.
If such empty tags are present at the top level, the service throws a "Bad Request" error. This was handled by the SDK to make sure that the top-level of the request body doesn't have any empty tags.
If such empty tags are present at the inner levels, the service assigns the empty strings as values to the empty tags instead of throwing an error. This wasn't handled and the empty tags at the rule filter created an unexpected filter for the subscription. This PR attempts to fix this issue.
### FIX
The key-value pairs having undefined/null as the values are removed recursively from the object that is being serialized.
Some more fixes related to the prior PRs around making BatchingReceiver behavior consistent:
* When we are closed (without error) or if we're in receiveAndDelete mode, we use the setTimeout() to ensure that we run after all pending message callbacks have fired
* We always cleanup our handlers before resolve/reject and it's made more clear if we're resolving immediately or resolving after pending message callbacks.
I also did some small refactors to make unit testing a bit easier, which should help make it simpler to validate this code path in the future.
Fixes#12922
Fixes#12901
### Purpose of change
Some users run their applications in a private network that lacks access to public endpoints, such as <namespace>.servicebus.windows.net. Due to these restrictions, these applications are unable to create direct connections to the Azure Event Hubs service using the standard endpoint address and require the ability to specify a custom host name to ensure they route through the proper intermediary for the connection to be made.
### Testing
I've added tests that verify the connection has been configured with the custom endpoint correctly.
I also manually ran these changes by setting up an Application Gateway in Azure with:
- backend pool with a backend target that points to the FQDN of my Event Hubs Namespace (`<namespace>.servicebus.windows.net`)
- A front-end listener using a HTTPS and a nonstandard port (200).
- HTTP settings with the following:
- backend port: 443
- Override with new host name: set to 'Yes'
- A rule that maps the front-end listener to the back-end target and uses the HTTP settings above.
I then had a test script that sent events, received events, and called operations on the $management link setting the `customEndpointAddress` to the public IP address from my application gateway.
Here's a simplified version of that script:
```js
const { EventHubProducerClient } = require('@azure/event-hubs');
const ws = require('ws');
const connectionString = `Endpoint=sb://<namespace>.servicebus.windows.net/;SharedAccessKeyName=sakn;SharedAccessKey=key`;
const eventHubName = `my-hub`;
async function run() {
const client = new EventHubProducerClient(connectionString, eventHubName, {
customEndpointAddress: `https://my.ip.address:200`,
webSocketOptions: {
webSocket: ws
}
});
const props = await client.getEventHubProperties();
console.log(props);
return client.close();
}
run().catch(console.error);
```
Note that in my application gateway I also used self-signed certs so needed to do some additional work so node.js would recognize my certificate, but that's separate from the customEndpointAddress work and doesn't require changes from our SDK.
This newly added command `docs` can help increase the quality of our documentation comments. It enables us to have a tight feedback loop on what is being generated as a documentation of our packages. I am pinning `typedoc` to v0.15.0 for now because this is the version being used for generating docs at `docs.microsoft.com`. This version should be updated when that team updates theirs.
Fixes https://github.com/Azure/azure-sdk-for-js/issues/12928
When we went down the 'drain' path with BatchingReceiver we'd actually remove the event listeners.
With the old code this could happen:
1. message received, debounce timer set
2. debounce timer fires
3. drain is requested (also removed the event listeners!)
4. message arrives (ignored)
5. receiver_drain arrives and resolve's the promise but the message in step 4 is lost.
The simplest way to reproduce this was to just connect over a slower link (created an SB in AU, which is slower than connecting over to something in the US) and send some about 100 messages (I used 2k-ish of text for each one) - this made it more likely that you'd end up with only a partial transfer of messages and have a drain. This became even more obvious once we moved to scheduling the promise resolution with setTimeout() as it widened the gap (this bug was less easy to see before!).
Many thanks to @enoeden for finding this bug!
Fixes#12711
* upgrade TS version and fix compilation issues
* upgrade the linting parser version and fix new linting issues
* fix cosmos sample
* address feedback
* fix linting issues in formrecognizer tests
* use unknown instead of any across our code
* address more issues
* cleanup package.json in core-http
* revert noisy linting changes caused by vanilla eslint rules not TS aware
* allow the poller to have results of type void
* fixing samples
* fix keyvault-certificates' sample
There were several issues causing the smoke tests to not be a useful signal - this PR improves on the situation and gets us back to green. There are still some issues that require followup which I'll be filing soon.
Fixes:
* ServiceBus was only whitelisting a single sample (usingAadAuth.js) which _used_ to pass but only because it wasn't doing any real work. When we changed it awhile back to actually attempt to use it's connection it failed. This still needs some investigation but in the meantime I've swapped it out and brought in some more useful samples like sendMessages.js, browseMessages.js and sessions.js, which should give us some coverage. This also required altering the test-resources.json so it properly created the sample queues as well as outputting them so they'd get set in the environment.
* FormRecognizer had some failing samples. After talking with @willmtemple I ignored one of them that will require some actual test data to be bootstrapped reasonably (so not necessarily a good candidate for this). I fixed another one that appeared to just not handle some data being empty (seems legitimate, but perhaps it's a bug).d up in the sample's environment.
Fixes#12803
Fixes an issue where drain could potentially resolve a promise even though some messages are pending processing.
This can happen if the user calls receiveMessages(5), for instance, and the timeout expires before they receive
them all - when this happens we then 'drain' the excess credits by sending a request to Service Bus on the link. Now, let's say that before the drain _response_ comes in we receive some of the other messages (after all, we requested 5). Those messages are then, via setTimeout(emit, 0), scheduled to be sent to our onMessage handler. They are not dispatched to us immediately.
Now, when the drain response arrives (prior to this fix) the drain is processed immediately (unlike other messages the rhea-promise code does not call it with setTimeout), potentially bypassing any onMessage callbacks that are still waiting to be processed in the task queue.
By using setTimeout() here we use the same technique that the messages are scheduled by, ensuring that the resolve() is processed after any of those pending callbacks.
Fixes#12711 (potentially)
* Add in section for dead letter queues
* Add docs links for the settlement methods
* Change our api ref to point to docs.microsoft.com now, rather than the blob generated ref docs.
Fixes#12260
The code that's removed is no longer necessary since the `retry` method is being called in a loop and the loop only calls the user's processError handler after the retries in a single `retry` call are exhausted.
Fixes#12559 and #12573
This change updates `ServiceBusError` in 2 ways.
1. Adds a constructor that accepts `message` and `code` directly. This lets us throw `ServiceBusError`s from the client without having to first create a `MessagingError`.
2. Update the constructor so that if a MessagingError's code is translated to `GeneralError`, the MessagingError's code is prefixed in the ServiceBusError message. This should provide additional context to the user on what went wrong.
This change updates the `ServiceBusError.code` field to match what was the `ServiceBusError.reason` field.
This allows us to remove the `MessagingErrorCodes` from the public interface, which we don't see as providing value above and beyond what the `ServiceBusCodes` provides.
It looks like the README.md still had a reference to the
`createSessionReciver` function which was removed in a previous preview.
I removed the reference to this method as the next two paragraphs
describe the two ways to actually create the session.
### Original issue - https://github.com/Azure/azure-sdk-for-js/issues/11108
Follow-up of the stress tests PR https://github.com/Azure/azure-sdk-for-js/pull/11546, here are the changes
- Uses the latest preview.8 version of the service-bus SDK
- Fixes the boolean options being passed as command-line args
- Multiple sessions support in the renew session lock test
- Adds (some)new options that can be passed as command-line args to the tests
- And maybe minor edits
Part of the list of breaking changes to core-amqp v2 in #12116
Replaces #12320 (precipitated by https://github.com/Azure/azure-sdk-for-js/pull/12320#discussion_r518438209)
This change moves the `DataTransformer` interface and `DefaultDataTransformer` class to service-bus and event-hub packages.
When we establish what our data serde strategy is, we can revisit using a shared common serde solution.
In #12216, we moved the code to settle a message and renew its lock from the ServiceBusMessageImpl to the ServiceBusReceiver. After this move, the only dependency left on the ConnectionContext in the ServiceBusMessageImpl is for the DataTransformer. This PR updates the ServiceBusMessage constructor to take the DataTransformer instead of the ConnectionContext and thus remove this dependency altogether.
This closes#9944 as we no longer need to track the context or the receiver on the message.
This closes#10620 as well. Since the context is not being tracked on the message anymore, it does not matter that the data transformer is tracked on the context.
Pick older core-http(~~1.1.6~~ 1.1.9) for service-bus preview.8 since 1.2.0 is not released.
This will be reverted once the service-bus preview.8 is released.
Next core-amqp release will be v2, updating files accordingly so that we can close#12266
Service Bus preview 8 is using core-amqp v2.0.0-beta.1 which has `MessageErrorCodes` and not `MessagingErrorCodes`. Updating SB to use `MessageErrorCodes` for now. Once SB preview 8 is released, we will update it to use v2.0.0 of core-amqp and revert this change
This PR ensures we do an early exit when input is empty array to methods that take an array as input
This closes#12095 which was logged for the `receiveDeferredMessages()` method and also fixes the same for `scheduleMessages()` and `cancelScheduledMessages()` calls
As seen in https://github.com/amqp/rhea/issues/323, `user-id` is wrongly typed in rhea.
This PR removes userId from the AMQP message properties in `@azure/core-amqp` package for now.
We will add it back once the type for `user-id` is settled on and rhea releases an update
This PR updates our samples as follows:
- Update comments in certain TS samples to match what is in the JS ones
- Update the sendMessages sample to use the batch
- Update the sendMessages sample in readme to use the batch
Closes#12118
`Message` from "rhea" library which was being exported as `AmqpMessage` is now being removed, `Message` from "rhea" library will be imported as `RheaMessage` in service-bus and event-hubs to adhere to the convention for "Amqp" prefix being followed with the `AmqpAnnotatedMessage` or `AmqpMessageProperties` which are defined in the core-amqp.
## Issue https://github.com/Azure/azure-sdk-for-js/issues/12004
### Previously
```ts
// Service Bus
AmqpAnnotatedMessage {
header: AmqpMessageHeader;
properties: AmqpMessageProperties;
}
AmqpMessageHeader {}
AmqpMessageProperties {}
```
```ts
// Core AMQP
Imports
MessageProperties as AmqpMessageProperties and
MessageHeader as AmqpMessageHeader
from rhea and re-exports them
(Have `underscore`s in the attributes instead of camel-case for the props from rhea)
// Exports new MessageProperties and new MessageHeader too (these are similar to AmqpMessage{Header|Properties} defined in service-bus)
```
### Updates
- ~~Re-exporting rhea types as `RheaAmqpMessage{Header|Properties}` instead of `AmqpMessage{Header|Properties}` from core-amqp~~ Removed these re-exports
- Renamed `Message{Header|Properties}` to `AmqpMessage{Header|properties}` in core-amqp (still being exported)
- Deleted the `AmqpMessage{Header|properties}` defined in service-bus and using `AmqpMessage{Header|Properties}` from core-amqp instead
- Re-exporting `AmqpMessage{Header|properties}` from core-amqp in service-bus
- Should we consider it non-breaking for core-amqp?
- API shape has been changed for core-amqp
- Event-hubs doesn't use any of the interfaces or types specified above, so it is not affected
- Service Bus V1 uses amqp-common, so not affected
- Only Service Bus V7 is affected(still in preview)
Updates the receiveMessagesStreaming sample to show how to handle errors. Also, fixed the existing samples so they're all up-to-date with the latest API
Fixes#12051, #8653, #7905
### Issue https://github.com/Azure/azure-sdk-for-js/issues/11860
We expect users to pass the modified properties object to the updateQueue method. However, we don't return just the properties object anywhere, we always return the properties wrapped in a response.
#### The intention behind this PR is to
- remove the users' confusion on what should be passed into the update methods
- give us more flexibility to add eTags support in a non-breaking way if the service gives better support for eTags!
Modifies `subscribe` to be more resilient against failures.
The general process is:
1. We always continually retry<> on init()
2. If a retry cycle fails (exhausts all retries configured via retryOptions) then we report the last error for that cycle.
3. We then begin a new cycle, running retry again.
The only way that a subscriber should quit is if the user passes and signals an AbortSignal.
This should make it much easier for people to write robust subscribe() implementations. A sample that shows how to handle some of the error codes will be coming shortly. This is merely the base.
Fix for #11719
Users that want the simplest and most robust experience possible are encouraged to user `Receiver.subscribe` when writing their receive loops. This breaks things down to just passing us a couple of callbacks.
As part of an overall effort this commit adds in some extra context information (mimicking what we're doing in the .NET ServiceBusProcessor) to let users make better decisions in their processError handler by passing in a `ProcessErrorArgs` object, which gives the user some extra context about where the error originated from (`errorSource`) and the host and entity path.
* fix MaxListenersExceeded for management client
* comment update
* Add listener for receiver_error inside createRheaLink instead of checking for the count before adding the listener
* Typo
* changelog
* formatting
* changelog - mention the warning
Jeremy Meng
Wenjie Yu（MSFT）
Will Temple
Timo van Veenendaal
Maor Leger
Deyaaeldeen Almahallawi
Jonathan Cárdenas
Harsha Nalluru
Ramya Rao
Azure SDK Bot
colawwj
Ben Broderick Phillips
Timo van Veenendaal
Hiroshi Yoshioka
Chidozie Ononiwu (His Righteousness)
chradek
Daniel Rodríguez
Richard Park
bashiMoha
Chidozie Ononiwu
KarishmaGhiya
Jose Manuel Heredia Hidalgo
Sima Zhu
Matt Ellis
Will Temple
Shrill Shrestha
Jeff Fisher
Adam Ling (MSFT)
Martin Ligtenberg
Mohsin Mehmood
Sima Zhu
Sree
Wes Haggard
