Skip to content

fix: "error while reseting"#512

Merged
NGTmeaty merged 2 commits into
mainfrom
ctx-reset
Oct 30, 2025
Merged

fix: "error while reseting"#512
NGTmeaty merged 2 commits into
mainfrom
ctx-reset

Conversation

@yzqzss

@yzqzss yzqzss commented Oct 28, 2025

Copy link
Copy Markdown
Collaborator

Issue: the global ctx has been cancelled during the shutdown process, ResetURL() has no chance to be issued.

@yzqzss yzqzss requested a review from Copilot October 28, 2025 18:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where URL reset operations were failing during the shutdown process because they were using a cancelled global context. The fix creates a new context with a 1-minute timeout specifically for the reset operations during shutdown.

Key Changes:

  • Creates a fresh context with timeout for reset operations during shutdown
  • Applies the fix to both LQ and HQ source implementations

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/pkg/source/lq/lq.go Creates new context with timeout for resetURL calls during Stop()
internal/pkg/source/hq/hq.go Creates new context with timeout for ResetURL calls during Stop()

Comment thread internal/pkg/source/lq/lq.go Outdated
Comment thread internal/pkg/source/hq/hq.go Outdated
@yzqzss yzqzss requested a review from NGTmeaty October 28, 2025 18:06
@codecov-commenter

codecov-commenter commented Oct 28, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.61%. Comparing base (33782fc) to head (1017da8).
⚠️ Report is 42 commits behind head on main.

Files with missing lines Patch % Lines
internal/pkg/source/hq/hq.go 0.00% 4 Missing ⚠️
internal/pkg/source/lq/lq.go 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #512      +/-   ##
==========================================
+ Coverage   56.36%   56.61%   +0.24%     
==========================================
  Files         130      130              
  Lines        8131     6481    -1650     
==========================================
- Hits         4583     3669     -914     
+ Misses       3184     2448     -736     
  Partials      364      364              
Flag Coverage Δ
e2etests 41.27% <25.00%> (+0.58%) ⬆️
unittests 29.14% <0.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NGTmeaty NGTmeaty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate it! Thank you!

@NGTmeaty NGTmeaty merged commit 2abbcd3 into main Oct 30, 2025
6 of 7 checks passed
@NGTmeaty NGTmeaty deleted the ctx-reset branch October 30, 2025 03:21
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