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

Issue #1428: exception when parsing timestamp #1429

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jbennett2091
Copy link

@jbennett2091 jbennett2091 commented Jan 22, 2021

Fixes #1428
If parsing fails, use default timestamp instead

@codecov
Copy link

codecov bot commented Jan 22, 2021

Codecov Report

Merging #1429 (a3a96fd) into master (e7fefda) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1429      +/-   ##
============================================
+ Coverage     57.69%   57.75%   +0.05%     
- Complexity     1916     1917       +1     
============================================
  Files           161      161              
  Lines          9006     9009       +3     
  Branches       1359     1359              
============================================
+ Hits           5196     5203       +7     
+ Misses         3335     3332       -3     
+ Partials        475      474       -1     
Impacted Files Coverage Δ Complexity Δ
.../fabric8/maven/docker/access/log/LogRequestor.java 84.52% <100.00%> (+5.51%) 14.00 <0.00> (+1.00)

@jbennett2091
Copy link
Author

Fixes #1428

Issue fabric8io#1428: Update LogRequestor.java to deal with stack trace

coverage tests

Signed-off-by: Jeffrey Bennett <jeffrey_bennett@ibi.com>
@jbennett2091
Copy link
Author

squashed and signed-off

@sonarcloud
Copy link

sonarcloud bot commented Jan 22, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@rhuss
Copy link
Collaborator

rhuss commented Mar 7, 2021

Thanks for the fix (and sorry for the delay) but I think we should also check, which the extraction of the timestamp fails in the first place. Happy to add a fallback (not sure about using the current timestamp though as there can be a considerable drift between the local time and the server time), but I suspect that the log line that causes the error matched the LOG_LINE regexp, but in an undesired way (i.e. the output of the log might have changed).

Could you check what concrete log line caused the error? Maybe the regexp could be adapt to pick up the concrete value. (btw the "Error" part in the exception provided in the original issue also looks a bit suspicious)

@jbennett2091
Copy link
Author

jbennett2091 commented Mar 8, 2021 via email

@rhuss
Copy link
Collaborator

rhuss commented Mar 8, 2021

yeah, no worries. I think we can more or less apply this fix but also log out a warning with the full log line in case we hit this. So hopefully we can fine tune the regexp in retrospective then.

@rhuss
Copy link
Collaborator

rhuss commented Jul 29, 2022

Sorry, there are conflict, not sure how to resolve those (it's very likely JMockit vs. Mockito ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception when parsing Timestamp
3 participants