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

Fixed an exception which happens when PutAsync is used more than once and activity logging is enabled in main project #675

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

uhfath
Copy link
Contributor

@uhfath uhfath commented Nov 11, 2024

Fixes #672

@@ -603,7 +603,7 @@ public async ValueTask DeleteAsync(string key, CancellationToken cancellationTok

private async ValueTask PublishMeta(ObjectMetadata meta, CancellationToken cancellationToken)
{
var ack = await JetStreamContext.PublishAsync(GetMetaSubject(meta.Name), meta, serializer: NatsObjJsonSerializer<ObjectMetadata>.Default, headers: NatsRollupHeaders, cancellationToken: cancellationToken);
var ack = await JetStreamContext.PublishAsync(GetMetaSubject(meta.Name), meta, serializer: NatsObjJsonSerializer<ObjectMetadata>.Default, headers: _natsRollupHeaders, cancellationToken: cancellationToken);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should create the headers here in the local scope. I have a feeling if you call put twice with telemetry turned on you'd still get the same exception even with this fix applied. Could you also write a test showing the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. Calling this method twice will result in same exception.
I'll make the changes.

@mtmk
Copy link
Collaborator

mtmk commented Nov 11, 2024

sorry @uhfath could you sign you commits please? you'd need to force push on top and feel free to squash into one commit.

… and activity logging is enabled in main project

Added some tests.
@uhfath
Copy link
Contributor Author

uhfath commented Nov 11, 2024

sorry @uhfath could you sign you commits please? you'd need to force push on top and feel free to squash into one commit.

Done.
I'm not sure why I wasn't able to run the tests locally but since they were trivial to write let's just hope they work as expected. 😊

@mtmk
Copy link
Collaborator

mtmk commented Nov 11, 2024

sorry @uhfath could you sign you commits please? you'd need to force push on top and feel free to squash into one commit.

Done. I'm not sure why I wasn't able to run the tests locally but since they were trivial to write let's just hope they work as expected. 😊

nice one. thanks for the extra tests. To run them locally you need a nats-server.exe in your PATH.

@mtmk mtmk self-assigned this Nov 12, 2024
Copy link
Collaborator

@mtmk mtmk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @uhfath

@mtmk mtmk merged commit 7dd7525 into nats-io:main Nov 13, 2024
10 checks passed
mtmk added a commit that referenced this pull request Nov 13, 2024
* Add OnSocketAvailableAsync Hook (#647)
* Refactor exception handling in NatsException (#677)
* Fixed an exception which happens when PutAsync is used more than once and activity logging is enabled in main project (#675)
* open Header.Writer class and create GetBytesLength method (#674)
* Fix date time serialization to use ISO8601 (#664)
@mtmk mtmk mentioned this pull request Nov 13, 2024
mtmk added a commit that referenced this pull request Nov 13, 2024
* Add OnSocketAvailableAsync Hook (#647)
* Refactor exception handling in NatsException (#677)
* Fixed an exception which happens when PutAsync is used more than once and activity logging is enabled in main project (#675)
* open Header.Writer class and create GetBytesLength method (#674)
* Fix date time serialization to use ISO8601 (#664)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InvalidOperationException during INatsObjStore.PutAsync
2 participants