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(connections): do not log AgentContext object #1085

Merged

Conversation

genaris
Copy link
Contributor

@genaris genaris commented Nov 4, 2022

In last Aries Framework JS call @niall-shaw reported performance issues when accepting multi-use invitations.

I've experienced the same issue after migrating to 0.3.0-alpha.x and it turns out that it was happening because some methods in DidExchangeProtocol were logging the whole messageContext object, which now includes agentContext and blocks the console for several seconds. This can be avoided by either logging in INFO mode or customizing the logger being in use to only output a certain depth level of objects.

This PR makes a quick-fix of this issue by logging only messageContext.message, to be consistent with other methods from ConnectionService. Not sure if it's really useful though, but that's another subject to discuss 😄 .

@genaris genaris requested a review from a team as a code owner November 4, 2022 22:42
@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2022

Codecov Report

Merging #1085 (64a9257) into main (ab403c9) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1085   +/-   ##
=======================================
  Coverage   88.34%   88.34%           
=======================================
  Files         701      701           
  Lines       16110    16110           
  Branches     2597     2597           
=======================================
  Hits        14232    14232           
  Misses       1871     1871           
  Partials        7        7           
Impacted Files Coverage Δ
...ore/src/modules/connections/DidExchangeProtocol.ts 86.40% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@berendsliedrecht
Copy link
Contributor

If this fixes the issue it is quite useful imo :).

@TimoGlastra TimoGlastra merged commit ef20f1e into openwallet-foundation:main Nov 7, 2022
NB-MikeRichardson added a commit to NB-MikeRichardson/aries-framework-javascript that referenced this pull request Nov 8, 2022
fix(connections): do not log AgentContext object (openwallet-foundation#1085)
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.

4 participants