Skip to content

refactor: use ExclusiveFetch to simplify AbortController - #40312

Merged
wxiaoguang merged 9 commits into
go-gitea:mainfrom
wxiaoguang:refactor-fetch
Nov 12, 2026
Merged

wxiaoguang merged 9 commits into
go-gitea:mainfrom
wxiaoguang:refactor-fetch

Conversation

@wxiaoguang

Copy link
Copy Markdown
Contributor

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 10, 2026
@github-actions github-actions Bot added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Nov 10, 2026
@silverwind

Copy link
Copy Markdown
Member

The ExclusiveFetch state handling looks correct. One regression in the Actions polling:

  1. Network failures of the 1s polls now show a sticky error toast, the old code ignored them on purpose (e instanceof TypeError). Firefox rejects in-flight fetches with NetworkError on navigation and download clicks, so "Download logs" or an artifact link on a running run leaves a toast behind, as does a single dropped poll. Fix: if (!details.response) return; at the top of both polling callbacks, which keeps the toast for preview and diff.
  2. loadCurrentRun no longer sends data: {}, so the binder reports Unsupported Content-Type on every summary poll. It only works because ViewPost ignores HasError. Fix: restore data: {}.

Written by Claude (Fable 5.2).

Comment thread web_src/js/modules/fetch.ts Resolved
Comment thread web_src/js/modules/fetch.ts Resolved
@wxiaoguang

wxiaoguang commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor Author
  1. Network failures of the 1s polls now show a sticky error toast,

If you dislike it, you can remove it. Or, hide it when next fetch succeeds. Update: see below #40312 (comment)

  1. loadCurrentRun no longer sends data: {},

Good catch. I have checked it when refactoring but it seems that I indeed missed it again

@wxiaoguang

Copy link
Copy Markdown
Contributor Author
  1. Network failures of the 1s polls now show a sticky error toast,

If you dislike it, you can remove it. Or, hide it when next fetch succeeds.

Added {duration: 2500} for error toasts.

Now the error message can tell end users why the loading fails, and won't bother end users for a too long time.

@wxiaoguang

Copy link
Copy Markdown
Contributor Author
  1. Network failures of the 1s polls now show a sticky error toast, the old code ignored them on purpose (e instanceof TypeError). Firefox rejects in-flight fetches with NetworkError on navigation and download clicks, so "Download logs" or an artifact link on a running run leaves a toast behind, as does a single dropped poll. Fix: if (!details.response) return; at the top of both polling callbacks, which keeps the toast for preview and diff.

0139da2 uses isNetworkError to ignore network errors. Such network errors seem useless to end users.

@silverwind

silverwind commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

State handling and polling look good, both earlier points are addressed.

  1. The new in-flight tracking has no test. If the finally reset breaks, both Actions pollers silently stop. Fix: in fetch.test.ts, add expect(exclusiveFetch.isInFlight()).toBe(false); right after the /new request.
  2. {duration: 2500} on the one-shot preview and diff toasts makes stale panel content look current once the toast fades. Suggestion: keep the sticky default there and the short duration only for the 1s polls.

Written by Claude (Opus 5.5).

@wxiaoguang

Copy link
Copy Markdown
Contributor Author
  • {duration: 2500} on the one-shot preview and diff toasts makes stale panel content look current once the toast fades. Suggestion: keep the sticky default there and the short duration only for the 1s polls.

I intentionally didn't use 1s. It will cause race and UI flash if the server error keep happening.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 12, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 12, 2026
@wxiaoguang
wxiaoguang added this pull request to the merge queue Nov 12, 2026
Merged via the queue into go-gitea:main with commit 37fa67f Nov 12, 2026
27 checks passed
@wxiaoguang
wxiaoguang deleted the refactor-fetch branch November 12, 2026 15:12
@GiteaBot GiteaBot added this to the 29.0.0 milestone Nov 12, 2026
silverwind added a commit to kirmanak/gitea that referenced this pull request Nov 12, 2026
…n-completed-workflow

* origin/main: (139 commits)
  refactor: use ExclusiveFetch to simplify AbortController (go-gitea#40312)
  fix: handle task list items checked with uppercase X (go-gitea#40334)
  test: add e2e tests for comments, file tree, actions and branch protection (go-gitea#40313)
  enhance(lfs): report status and body on non-JSON LFS error responses (go-gitea#40331)
  feat(ssh)!: remove SSH DSA key support (go-gitea#40332)
  refactor(git): parse git stdout correctly (go-gitea#40326)
  [skip ci] Updated translations via Crowdin
  fix(repo): rank exact repository name matches first (go-gitea#39038)
  fix(web): avoid compare output route collisions (go-gitea#40307)
  refactor: clean up error system (go-gitea#40328)
  fix(markup): keep list items on separate lines in quote replies (go-gitea#40329)
  fix(repo): require the unit in AccessibleRepositoryCondition (go-gitea#40059)
  fix(ui): consistent spacing between labels (go-gitea#40261)
  [skip ci] Updated translations via Crowdin
  test: fix the login tab order e2e test in macOS WebKit (go-gitea#40316)
  fix(api): keep webhook settings that are omitted in an edit request (go-gitea#40282)
  fix(indexer): reindex issues when their projects change (go-gitea#40299)
  chore(deps): update go toolchain directive to v1.27.4 (go-gitea#40315)
  chore(css): remove dead CSS (go-gitea#40314)
  fix(packages): generic package deletion ignores terraform state lock and latest version (go-gitea#40302)
  ...

Assisted-by: Claude Code:claude-opus-5-5

# Conflicts:
#	modelmigration/migrations.go
#	modelmigration/v29/v365.go
#	modelmigration/v29/v365_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants