fix(watcher): skip context when Microcks client cannot be created - #482
fix(watcher): skip context when Microcks client cannot be created#482SAY-5 wants to merge 1 commit into
Conversation
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
This pull request has been automatically marked as stale because it has not had recent activity 😴 It will be closed in 30 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation. There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. Microcks is a Cloud Native Computing Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model. Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here. Thank you for your patience ❤️ |
|
Thank you @aniket866 This issue has been fixed on master — TriggerImport now continues to the next context when connectors.NewClient fails, so the nil client can never reach UploadArtifact. I believe this can be closed. |
|
Same comment: #496 (comment) |
|
You're absolutely right, @Vaishnav88sk, thanks for checking, and apologies for the noise. I re-read the full control flow and my comment was wrong: the continue only exists in the else branch (no-config path via NewMicrocksClient). The config-path branch logs the NewClient error and falls through to mc.UploadArtifact on a nil client — exactly the #475 scenario. So #475 is still valid and this PR still applies. |
Thanks for the clarification. Could you please recheck the comments yu made previously on issues/PRs about the closure. I think there are some that not yet solved or needed. |
Done — I went back over every comment I posted, re-verifying each against master with full code reads this time. Everything held up so far: importDirectoryPartialFailure for #473/#379, reference-counting deleteContext for #421/#427, ResolveContext in logout for #419, waitForReady in start for #303/#311, ExactArgs + --output in test for #446/#332, resp.Body + regression test for #337/#375, and zero panic( sites left in pkg/connectors for the #489 family. One refinement rather than a correction: the "client_secret gap" I noted on #265/#333 turned out to be bigger — it's now properly documented in #503 (JSON bodies bypass redaction), with a fix in review at #504. I am still reviewing pending ones which I will continue later |
Problem
In
TriggerImport, whenconnectors.NewClientfails it logs the error butkeeps going, so the code reaches
mc.UploadArtifactwith a nil client and thewatcher process panics with a nil pointer dereference (#475). This happens for
example when a watch entry references a context that cannot be resolved.
Fix
continueto the next context after logging the connection error, so a failedcontext is skipped instead of crashing the daemon.
Added a regression test in
pkg/watcherthat drivesTriggerImportwith anunresolvable context; it panics without the change and passes with it.
Fixes #475