Fix/ksef sync error logging#62
Merged
Merged
Conversation
Log HTTP method, full URL, status code, and truncated response body on every error path across all 8 KSeF API functions. Also strip trailing slash from KSEF_API_URL to prevent double-slash URLs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each auth step (get_challenge, sign_challenge, authenticate_xades, poll_auth_status, redeem_tokens) now logs its name and duration in ms, making it easy to pinpoint which step fails in production. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Helps verify the correct environment is targeted when diagnosing production sync failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds NIP to reauth_failed and generic error logs for easier cross-referencing with KSeF API logs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (3)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip You can customize the tone of the review comments and chat replies.Configure the 📝 WalkthroughWalkthroughThis PR adds comprehensive logging and instrumentation across multiple KSeF client modules to improve observability. Changes include startup-time API URL logging, step-based authentication instrumentation with timing, extensive HTTP response logging with error handling, and enhanced sync worker error messages with company NIP context. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/ksef_hub/ksef_client/auth.ex (1)
1-1:⚠️ Potential issue | 🟡 MinorRun
mix formatto fix the pipeline failure.The CI pipeline is failing because this file is not properly formatted. All four changed files need formatting.
Fix all formatting issues at once
mix format lib/ksef_hub/application.ex lib/ksef_hub/sync/sync_worker.ex lib/ksef_hub/ksef_client/live.ex lib/ksef_hub/ksef_client/auth.ex🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub/ksef_client/auth.ex` at line 1, The CI failed due to formatting; run mix format on the affected files and ensure the KsefHub.KsefClient.Auth module (and the other changed modules) are formatted: execute the provided command to format lib/ksef_hub/application.ex, lib/ksef_hub/sync/sync_worker.ex, lib/ksef_hub/ksef_client/live.ex and lib/ksef_hub/ksef_client/auth.ex so the module definitions (e.g., defmodule KsefHub.KsefClient.Auth) and pipelines comply with mix format.lib/ksef_hub/ksef_client/live.ex (1)
1-9:⚠️ Potential issue | 🟡 MinorRun
mix formatto fix the pipeline failure.The CI pipeline is failing because this file is not properly formatted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ksef_hub/ksef_client/live.ex` around lines 1 - 9, The file KsefHub.KsefClient.Live (module KsefHub.KsefClient.Live) is not formatted correctly; run the Elixir formatter on this file by executing mix format (or format the project) so the module definition, `@moduledoc`, `@behaviour` and require Logger lines conform to the project's formatting rules and fix the CI pipeline failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ksef_hub/sync/sync_worker.ex`:
- Around line 292-298: Run the Elixir formatter to fix CI failure: apply mix
format to the module containing lookup_company_nip/1 (the `@spec` and defp
lookup_company_nip function) so the spacing and pipeline formatting conform to
project style; you can run `mix format` locally or format that function block to
match the repo conventions.
---
Outside diff comments:
In `@lib/ksef_hub/ksef_client/auth.ex`:
- Line 1: The CI failed due to formatting; run mix format on the affected files
and ensure the KsefHub.KsefClient.Auth module (and the other changed modules)
are formatted: execute the provided command to format
lib/ksef_hub/application.ex, lib/ksef_hub/sync/sync_worker.ex,
lib/ksef_hub/ksef_client/live.ex and lib/ksef_hub/ksef_client/auth.ex so the
module definitions (e.g., defmodule KsefHub.KsefClient.Auth) and pipelines
comply with mix format.
In `@lib/ksef_hub/ksef_client/live.ex`:
- Around line 1-9: The file KsefHub.KsefClient.Live (module
KsefHub.KsefClient.Live) is not formatted correctly; run the Elixir formatter on
this file by executing mix format (or format the project) so the module
definition, `@moduledoc`, `@behaviour` and require Logger lines conform to the
project's formatting rules and fix the CI pipeline failure.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/ksef_hub/application.exlib/ksef_hub/ksef_client/auth.exlib/ksef_hub/ksef_client/live.exlib/ksef_hub/sync/sync_worker.ex
Comment on lines
+292
to
+298
| @spec lookup_company_nip(Ecto.UUID.t()) :: String.t() | ||
| defp lookup_company_nip(company_id) do | ||
| case KsefHub.Companies.get_company(company_id) do | ||
| %{nip: nip} -> nip | ||
| _ -> "unknown" | ||
| end | ||
| end |
There was a problem hiding this comment.
Run mix format to fix the pipeline failure.
The CI pipeline is failing due to formatting. The lookup_company_nip/1 implementation is clean—using pattern matching and a sensible fallback. NIP (tax ID) is a business identifier that aids debugging without exposing sensitive credentials.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ksef_hub/sync/sync_worker.ex` around lines 292 - 298, Run the Elixir
formatter to fix CI failure: apply mix format to the module containing
lookup_company_nip/1 (the `@spec` and defp lookup_company_nip function) so the
spacing and pipeline formatting conform to project style; you can run `mix
format` locally or format that function block to match the repo conventions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit