Page MenuHomePhabricator
Differential D13541

[Chronik] Add \n to logs in error.rs
ClosedPublic
Actions

Authored by tobias_ruck on Apr 3 2023, 13:46.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCad3228289f0b: [Chronik] Add \n to logs in error.rs
Summary

These look messy in debug.log when they occur because of the missing \n at the end.

For example, test plan on master would result in [chronik/chronik-http/src/error.rs:39] [<chronik unknown>] helloChronik HTTP server got an unknown error: helloReceived a POST request for
(cringe).

Now the log looks like this (based):

2023-04-03T13:40:31.058558Z (mocktime: 2012-04-02T02:22:17Z) [chronik/chronik-http/src/error.rs:39] [<chronik unknown>] hello
2023-04-03T13:40:31.058609Z (mocktime: 2012-04-02T02:22:17Z) [chronik/chronik-http/src/error.rs:40] [<chronik unknown>] Chronik HTTP server got an unknown error: hello
2023-04-03T13:40:31.116991Z (mocktime: 2012-04-02T02:22:17Z) [src/httpserver.cpp:252] [http_request_cb] Received a POST request for / from 127.0.0.1:62544
Test Plan
  1. Add abc_rust_error::bail!("hello") as first line in QueryTxs::tx_by_id
  2. ninja
  3. run chronik_tx functional test (-> fails)
  4. logs log multiple lines now

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tobias_ruck created this revision.Apr 3 2023, 13:46
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 3 2023, 13:46
tobias_ruck requested review of this revision.Apr 3 2023, 13:46
Harbormaster completed remote builds in B22913: Diff 39159.Apr 3 2023, 14:00
Fabien requested changes to this revision.Apr 3 2023, 15:58

That's the wrong changes

This revision now requires changes to proceed.Apr 3 2023, 15:58
tobias_ruck added a comment.Apr 3 2023, 16:54

lmao what

tobias_ruck updated this revision to Diff 39170.Apr 3 2023, 17:03

diff the right changes

Harbormaster completed remote builds in B22920: Diff 39170.Apr 3 2023, 17:17
Fabien accepted this revision.Apr 4 2023, 08:07

We should extend the log linter to catch these

This revision is now accepted and ready to land.Apr 4 2023, 08:07
Closed by commit rABCad3228289f0b: [Chronik] Add \n to logs in error.rs (authored by tobias_ruck). · Explain WhyApr 4 2023, 09:28
This revision was automatically updated to reflect the committed changes.
tobias_ruck added a commit: rABCad3228289f0b: [Chronik] Add \n to logs in error.rs.

Revision Contents
Changeset List

PathSize
chronik/
chronik-http/
src/
8 lines

Diff 39200

View Options

chronik/chronik-http/src/error.rs

Loading...