startLocationUpdatesAsyncに指数バックオフ付きリトライ処理を追加#5401
Merged
TinyKitten merged 5 commits intodevfrom Feb 21, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughstartLocationUpdatesAsync 呼び出しに最大3回の再試行（基底遅延1000msでのバックオフ）を追加し、アンマウント時のキャンセルと再試行／停止失敗の警告ログを導入。関連定数を追加し、再試行を検証するテストを拡張。 変更内容
Sequence Diagram(s)sequenceDiagram
participant Hook as Hook (useStartBackgroundLocationUpdates)
participant LocationAPI as Expo Location API
participant Timer as Backoff Timer
participant Cleanup as Component Unmount
Hook->>LocationAPI: startLocationUpdatesAsync()
alt success
LocationAPI-->>Hook: resolved
else failure
LocationAPI-->>Hook: rejected (error)
Hook->>Hook: console.warn("retry n / max")
Hook->>Timer: wait(backoffDelay)
Timer-->>Hook: timer done
alt cancelled (Unmount happened)
Cleanup->>Hook: set cancelled=true
Hook->>LocationAPI: stopLocationUpdatesAsync() (if started)
LocationAPI-->>Hook: resolved/rejected
else retry
Hook->>LocationAPI: startLocationUpdatesAsync() (retry n+1)
end
end
推定コードレビュー工数🎯 4 (Complex) | ⏱️ ~45分 関連する可能性のあるプルリクエスト
詩
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (1)
src/hooks/useStartBackgroundLocationUpdates.test.tsx (1)
39-45:⚠️ Potential issue | 🟡 Minorjest.clearAllMocks は afterEach で実行してください。
As per coding guidelines, "Call `jest.clearAllMocks()` in `afterEach` blocks in all test files".🧹 修正案
beforeEach(() => { - jest.clearAllMocks(); mockAutoModeEnabled = false; mockStartLocationUpdatesAsync.mockResolvedValue(undefined); mockStopLocationUpdatesAsync.mockResolvedValue(undefined); mockWatchPositionAsync.mockResolvedValue({ remove: mockRemove }); }); + + afterEach(() => { + jest.clearAllMocks(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 39 - 45, Move the call to jest.clearAllMocks() out of the beforeEach and into an afterEach block; keep the mock setup (mockAutoModeEnabled = false, mockStartLocationUpdatesAsync.mockResolvedValue(undefined), mockStopLocationUpdatesAsync.mockResolvedValue(undefined), mockWatchPositionAsync.mockResolvedValue({ remove: mockRemove })) in beforeEach, and add an afterEach that calls jest.clearAllMocks() to conform to the guideline; adjust the test file so only teardown clears mocks while initialization remains in the beforeEach that references mockAutoModeEnabled, mockStartLocationUpdatesAsync, mockStopLocationUpdatesAsync, mockWatchPositionAsync and mockRemove.
🧹 Nitpick comments (1)
src/hooks/useStartBackgroundLocationUpdates.test.tsx (1)
3-7: リトライ待機時間は定数参照にすると保守しやすいです。♻️ 修正案
import { LOCATION_START_MAX_RETRIES, + LOCATION_START_RETRY_BASE_DELAY_MS, LOCATION_TASK_NAME, LOCATION_TASK_OPTIONS, } from '../constants'; @@ - await jest.advanceTimersByTimeAsync(1000); + await jest.advanceTimersByTimeAsync( + LOCATION_START_RETRY_BASE_DELAY_MS + ); @@ - await jest.advanceTimersByTimeAsync(1000 * 2 ** i); + await jest.advanceTimersByTimeAsync( + LOCATION_START_RETRY_BASE_DELAY_MS * 2 ** i + );Also applies to: 126-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 3 - 7, Tests in useStartBackgroundLocationUpdates.test.tsx use hardcoded retry-wait numbers; replace those literals with the retry-wait constant exported from '../constants' (alongside the already-imported LOCATION_START_MAX_RETRIES, LOCATION_TASK_NAME, LOCATION_TASK_OPTIONS) so the tests reference the canonical value and stay in sync — update assertions, mocked timers/delays and any wait-loop logic in the useStartBackgroundLocationUpdates test cases (including the other block noted) to use that constant and add the import if it’s not present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useStartBackgroundLocationUpdates.ts`:
- Around line 31-73: The async start loop in useStartBackgroundLocationUpdates
may start location updates after the hook was cancelled; after a successful call
to Location.startLocationUpdatesAsync(LOCATION_TASK_NAME) re-check the cancelled
flag and, if true, immediately call and await
Location.stopLocationUpdatesAsync(LOCATION_TASK_NAME) to ensure no leftover
updates run; update the code around the startLocationUpdatesAsync call (and its
surrounding try block) to perform this cancelled check and stop, referencing the
existing cancelled variable, LOCATION_TASK_NAME, startLocationUpdatesAsync and
stopLocationUpdatesAsync to locate where to add the guard.
---
Outside diff comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 39-45: Move the call to jest.clearAllMocks() out of the beforeEach
and into an afterEach block; keep the mock setup (mockAutoModeEnabled = false,
mockStartLocationUpdatesAsync.mockResolvedValue(undefined),
mockStopLocationUpdatesAsync.mockResolvedValue(undefined),
mockWatchPositionAsync.mockResolvedValue({ remove: mockRemove })) in beforeEach,
and add an afterEach that calls jest.clearAllMocks() to conform to the
guideline; adjust the test file so only teardown clears mocks while
initialization remains in the beforeEach that references mockAutoModeEnabled,
mockStartLocationUpdatesAsync, mockStopLocationUpdatesAsync,
mockWatchPositionAsync and mockRemove.
---
Nitpick comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 3-7: Tests in useStartBackgroundLocationUpdates.test.tsx use
hardcoded retry-wait numbers; replace those literals with the retry-wait
constant exported from '../constants' (alongside the already-imported
LOCATION_START_MAX_RETRIES, LOCATION_TASK_NAME, LOCATION_TASK_OPTIONS) so the
tests reference the canonical value and stay in sync — update assertions, mocked
timers/delays and any wait-loop logic in the useStartBackgroundLocationUpdates
test cases (including the other block noted) to use that constant and add the
import if it’s not present.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useStartBackgroundLocationUpdates.ts (1)
29-83:⚠️ Potential issue | 🟡 Minorキャンセル後の stop 失敗が start 失敗扱いになる点を分離してください
cancelled分岐内の stop が失敗するとcatchに流れて「開始失敗」ログ＆待機が走るため、停止失敗は別ログで握りつぶして即 return する方が意図が明確です。🛠️ 修正案
if (cancelled) { - await Location.stopLocationUpdatesAsync(LOCATION_TASK_NAME); + await Location.stopLocationUpdatesAsync(LOCATION_TASK_NAME).catch( + (stopError) => { + console.warn( + 'バックグラウンド位置情報の更新停止に失敗しました:', + stopError + ); + } + ); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.ts` around lines 29 - 83, The start routine currently calls await Location.stopLocationUpdatesAsync(LOCATION_TASK_NAME) inside the try block when cancelled is true, and any error there bubbles to the surrounding catch which treats it as a start failure (triggering retry logic); change this by wrapping the cancelled-path stop call in its own try/catch so stop errors are logged separately (e.g. "停止失敗") and swallowed, then immediately return without letting the error propagate to the outer catch; update the async IIFE around startLocationUpdatesAsync/stopLocationUpdatesAsync (referencing cancelled, startLocationUpdatesAsync, stopLocationUpdatesAsync, LOCATION_TASK_NAME, and the existing catch/attempt/LOCATION_START_MAX_RETRIES logic) so retries only run for start failures and not for stop failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/hooks/useStartBackgroundLocationUpdates.ts`:
- Around line 29-83: The start routine currently calls await
Location.stopLocationUpdatesAsync(LOCATION_TASK_NAME) inside the try block when
cancelled is true, and any error there bubbles to the surrounding catch which
treats it as a start failure (triggering retry logic); change this by wrapping
the cancelled-path stop call in its own try/catch so stop errors are logged
separately (e.g. "停止失敗") and swallowed, then immediately return without letting
the error propagate to the outer catch; update the async IIFE around
startLocationUpdatesAsync/stopLocationUpdatesAsync (referencing cancelled,
startLocationUpdatesAsync, stopLocationUpdatesAsync, LOCATION_TASK_NAME, and the
existing catch/attempt/LOCATION_START_MAX_RETRIES logic) so retries only run for
start failures and not for stop failures.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useStartBackgroundLocationUpdates.test.tsx (1)
39-45:⚠️ Potential issue | 🟡 MinorafterEach で jest.clearAllMocks を呼ぶようにしてください。
beforeEach のみだとガイドライン違反です。As per coding guidelines: Call `jest.clearAllMocks()` in `afterEach` blocks in all test files.修正案
beforeEach(() => { - jest.clearAllMocks(); mockAutoModeEnabled = false; mockStartLocationUpdatesAsync.mockResolvedValue(undefined); mockStopLocationUpdatesAsync.mockResolvedValue(undefined); mockWatchPositionAsync.mockResolvedValue({ remove: mockRemove }); }); + + afterEach(() => { + jest.clearAllMocks(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx` around lines 39 - 45, Add a teardown that calls jest.clearAllMocks() in an afterEach block for the test file: ensure afterEach(() => { jest.clearAllMocks(); }) is present alongside the existing beforeEach setup that initializes mockAutoModeEnabled, mockStartLocationUpdatesAsync, mockStopLocationUpdatesAsync, and mockWatchPositionAsync; keep the existing mockResolvedValue calls for mockStartLocationUpdatesAsync, mockStopLocationUpdatesAsync, and mockWatchPositionAsync but move/duplicate the jest.clearAllMocks call into afterEach so mocks are cleared after each test run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/hooks/useStartBackgroundLocationUpdates.test.tsx`:
- Around line 39-45: Add a teardown that calls jest.clearAllMocks() in an
afterEach block for the test file: ensure afterEach(() => {
jest.clearAllMocks(); }) is present alongside the existing beforeEach setup that
initializes mockAutoModeEnabled, mockStartLocationUpdatesAsync,
mockStopLocationUpdatesAsync, and mockWatchPositionAsync; keep the existing
mockResolvedValue calls for mockStartLocationUpdatesAsync,
mockStopLocationUpdatesAsync, and mockWatchPositionAsync but move/duplicate the
jest.clearAllMocks call into afterEach so mocks are cleared after each test run.
This was referenced Feb 22, 2026
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
2 participants
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.
https://claude.ai/code/session_011UAtpQJK1XnENVkcaQRmMi
Summary by CodeRabbit
バグ修正
テスト