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

fix: internal server error on exchange query #117

Merged

Conversation

lenkan
Copy link
Collaborator

@lenkan lenkan commented Oct 23, 2023

Fixes #115

@pfeairheller I just did the same thing you did in ExchangeResourceEnd from #116. This happened on both ExchangeResourceEnd and ExchangeQueryCollectionEnd.

Copy link
Member

@m00sey m00sey left a comment

Choose a reason for hiding this comment

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

can we get a test that highlighted what was broken?

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #117 (b62429d) into development (117a1ca) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           development     #117   +/-   ##
============================================
  Coverage        91.87%   91.87%           
============================================
  Files               35       35           
  Lines             5671     5671           
============================================
  Hits              5210     5210           
  Misses             461      461           
Files Coverage Δ
src/keria/peer/exchanging.py 90.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lenkan
Copy link
Collaborator Author

lenkan commented Oct 23, 2023

@m00sey I would like to try, but I could not find an example of a test that verified the same fix here: https://github.com/WebOfTrust/keria/pull/116/files#diff-c01bb1f6f142fd2e0b1a4f6afc37afabd7c247b268ee85d8e3c1f0f649bc22f9R169 on the ExchangeResourceEnd get method.

I can try to see if I can understand the current test_exchanging test. It would be great if it was split up into smaller pieces.

@lenkan
Copy link
Collaborator Author

lenkan commented Oct 23, 2023

@m00sey I tried to add a test here but I could not figure out how to add attachments to an exchange message in the test.

https://github.com/lenkan/keria/blob/103d8c9f97afa5bbdef72887414f4c795ca9a79a/tests/peer/test_exchanging.py#L149-L189

Do you have any pointers?

@m00sey m00sey self-assigned this Oct 26, 2023
@pfeairheller
Copy link
Member

I'll merge this and then submit a PR with a test.

@pfeairheller pfeairheller merged commit 35f9ed5 into WebOfTrust:development Oct 30, 2023
5 checks passed
@lenkan lenkan deleted the fix-non-serializable-bytearray branch January 8, 2024 12:29
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.

Internal server error on exchanges query after IPEX grant message
3 participants