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

log response, particularly in case of failure #1314

Merged
merged 4 commits into from
Oct 29, 2021
Merged

Conversation

hassanctech
Copy link
Contributor

@hassanctech hassanctech commented Oct 29, 2021

Issue #, if available:

Description of changes:

For example before if the request signature was too old we should just swallow that error and manifest as a timeout 0x0000000f in the sync call. Now we have logging to indicate what the error is, for example:

2021-10-29 14:50:08 WARN    lwsHttpCallbackRoutine(): Received client http read response:  {"message":"Signature expired: 20211029T144008Z is now earlier than 20211029T144508Z (20211029T145008Z - 5 min.)"}

In this instance I tried to submit the API call 10 minutes after the time was set in the request header.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-commenter
Copy link

codecov-commenter commented Oct 29, 2021

Codecov Report

Merging #1314 (03c007b) into master (d08634b) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1314      +/-   ##
==========================================
- Coverage   86.01%   85.81%   -0.20%     
==========================================
  Files          42       42              
  Lines       10594    10600       +6     
==========================================
- Hits         9112     9096      -16     
- Misses       1482     1504      +22     
Impacted Files Coverage Δ
src/source/Signaling/LwsApiCalls.c 84.06% <100.00%> (+0.06%) ⬆️
src/source/Ice/SocketConnection.c 79.72% <0.00%> (-4.06%) ⬇️
src/source/Ice/IceUtils.c 96.03% <0.00%> (-1.59%) ⬇️
src/source/Ice/IceAgentStateMachine.c 83.90% <0.00%> (-1.37%) ⬇️
src/source/PeerConnection/jsmn.h 75.93% <0.00%> (-0.58%) ⬇️
src/source/Ice/IceAgent.c 94.59% <0.00%> (-0.42%) ⬇️
src/source/Ice/ConnectionListener.c 92.63% <0.00%> (-0.04%) ⬇️
src/source/Ice/TurnConnection.c 93.46% <0.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d08634b...03c007b. Read the comment docs.

@hassanctech hassanctech changed the title add the call response, it's verbose but maybe we should make it debug log response, particularly in case of failure Oct 29, 2021
src/source/Signaling/LwsApiCalls.c Show resolved Hide resolved
@hassanctech hassanctech merged commit 4127014 into master Oct 29, 2021
@hassanctech hassanctech deleted the log-call-response branch October 29, 2021 21:14
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