Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 124 additions & 35 deletions src/background/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,28 @@ import { generateAnswersWithClaudeWebApi } from '../services/apis/claude-web.mjs
import { generateAnswersWithMoonshotWebApi } from '../services/apis/moonshot-web.mjs'
import { isUsingModelName } from '../utils/model-name-convert.mjs'
import { redactSensitiveFields } from './redact.mjs'
import {
clearProxyReconnectErrorSuppression,
consumeProxyReconnectErrorSuppression,
forwardProxyMessage,
interruptProxyGeneration,
isCurrentProxyGenerationMessage,
markProxyGenerationFinished,
markProxyGenerationFinishedFromMessage,
markProxyGenerationStarted,
shouldSkipProxyReconnect,
tagProxyRequestGeneration,
} from './proxy-generation-state.mjs'

function postProxySession(port, session, requestGenerationId) {
const proxyGenerationId = (port._proxyGenerationId ?? 0) + 1
port.proxy.postMessage({
session,
proxyGenerationId,
...(requestGenerationId === undefined ? {} : { requestGenerationId }),
})
markProxyGenerationStarted(port, proxyGenerationId, requestGenerationId)
Comment on lines +64 to +71

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.

Action required

1. Stale disconnect requestid 🐞 Bug ≡ Correctness

postProxySession() posts to port.proxy before recording the new request generation state, but
the proxy disconnect handler tags its {done:true, proxyDisconnected:true} using the stored
_proxyRequestGenerationId. If port.proxy.postMessage() throws or a disconnect happens before
markProxyGenerationStarted() runs, the disconnect completion can be tagged with a previous
requestGenerationId and be dropped by the UI’s superseded-request filter, leaving the current
request stuck.
Agent Prompt
### Issue description
`postProxySession()` currently calls `port.proxy.postMessage(...)` before persisting the new request’s generation bookkeeping via `markProxyGenerationStarted(...)`. Disconnect/error paths (e.g. proxy disconnect completion) rely on `port._proxyRequestGenerationId` when tagging messages; if the proxy post fails or disconnect occurs before the bookkeeping is committed, the emitted completion can be tagged with a stale `requestGenerationId` and dropped by the UI.

### Issue Context
- Disconnect completion is tagged using `tagProxyRequestGeneration(port, ...)`, which reads `port._proxyRequestGenerationId`.
- The UI drops messages whose `requestGenerationId` doesn’t match the current request.

### Fix Focus Areas
- src/background/index.mjs[64-72]
- src/background/index.mjs[184-200]
- src/background/proxy-generation-state.mjs[1-12]

### Suggested fix
Make the request/proxy generation state update atomic with respect to disconnect/error tagging:
1) Move the state update ahead of `port.proxy.postMessage(...)` (or at least set `_proxyRequestGenerationId` and `_proxyGenerationId` before posting).
2) If you need `_generating` to reflect only *successful* dispatch, consider:
   - pre-setting IDs used for tagging (request/proxy generation ids),
   - then setting `_generating=true` only after a successful post,
   - and reverting/clearing IDs if the post fails.
3) Add/adjust unit tests to cover the failure path where `postMessage` throws and a subsequent disconnect/max-retry path emits a tagged message (ensuring it uses the current requestGenerationId).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

const RECONNECT_CONFIG = {
MAX_ATTEMPTS: 5,
Expand Down Expand Up @@ -111,6 +133,11 @@ function setPortProxy(port, proxyTabId) {
console.debug('[background] Main port closed; skipping proxy message.')
return
}
if (!isCurrentProxyGenerationMessage(port, msg)) {
console.debug('[background] Ignoring a message from a superseded proxy generation.')
return
}
markProxyGenerationFinishedFromMessage(port, msg)
try {
port.postMessage(msg)
} catch (e) {
Expand All @@ -126,27 +153,30 @@ function setPortProxy(port, proxyTabId) {
console.debug('[background] Message to proxy tab (redacted):', redactedMsg)
if (port.proxy) {
try {
port.proxy.postMessage(msg)
forwardProxyMessage(port, msg)
} catch (e) {
console.error(
'[background] Error posting message to proxy tab in _portOnMessage:',
e,
redactedMsg,
)
try {
// Attempt to notify the original sender about the failure
port.postMessage({
error:
'Failed to forward message to target tab. Tab might be closed or an extension error occurred.',
})
} catch (notifyError) {
console.error(
'[background] Error sending forwarding failure notification back to original sender:',
notifyError,
)
if (!msg?.stop) {
try {
// Attempt to notify the original sender about the failure
port.postMessage({
error:
'Failed to forward message to target tab. Tab might be closed or an extension error occurred.',
})
} catch (notifyError) {
console.error(
'[background] Error sending forwarding failure notification back to original sender:',
notifyError,
)
}
}
}
} else {
if (msg?.stop) interruptProxyGeneration(port)
console.warn('[background] Port proxy not available to send message:', redactedMsg)
}
}
Expand All @@ -157,6 +187,17 @@ function setPortProxy(port, proxyTabId) {
const proxyRef = port.proxy
port.proxy = null
port._proxyTabId = null
if (interruptProxyGeneration(port)) {
if (!port._isClosed) {
try {
port.postMessage(
tagProxyRequestGeneration(port, { done: true, proxyDisconnected: true }),
)
} catch (e) {
console.warn('[background] Error posting done on proxy disconnect:', e)
}
}
}
if (port._reconnectTimerId) {
clearTimeout(port._reconnectTimerId)
port._reconnectTimerId = null
Expand Down Expand Up @@ -208,12 +249,20 @@ function setPortProxy(port, proxyTabId) {
console.warn('[background] Error removing _portOnDisconnect on max retries:', e)
}
}
try {
port.postMessage({
error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.`,
})
} catch (e) {
console.warn('[background] Error sending final error message on max retries:', e)
if (consumeProxyReconnectErrorSuppression(port)) {
console.debug(
'[background] Skipping reconnect error because the interrupted generation was already completed.',
)
} else {
try {
port.postMessage(
tagProxyRequestGeneration(port, {
error: `Connection to ChatGPT tab lost after ${RECONNECT_CONFIG.MAX_ATTEMPTS} attempts. Please refresh the page.`,
}),
)
} catch (e) {
console.warn('[background] Error sending final error message on max retries:', e)
}
}
return
}
Expand Down Expand Up @@ -242,6 +291,10 @@ function setPortProxy(port, proxyTabId) {
)
return
}
if (shouldSkipProxyReconnect(port)) {
console.debug('[background] Proxy already replaced; skipping stale reconnect callback.')
return
}
console.debug(
`[background] Retrying connection to tab ${proxyTabId}, attempt ${port._reconnectAttempts}.`,
)
Expand All @@ -258,6 +311,7 @@ function setPortProxy(port, proxyTabId) {
'[background] Main port disconnected (e.g. popup/sidebar closed). Cleaning up proxy connections and listeners.',
)
port._isClosed = true
markProxyGenerationFinished(port)
if (port._reconnectTimerId) {
clearTimeout(port._reconnectTimerId)
port._reconnectTimerId = null
Expand Down Expand Up @@ -330,6 +384,7 @@ function setPortProxy(port, proxyTabId) {
port._reconnectAttempts = 0
console.debug('[background] Reset reconnect attempts after stable proxy connection.')
}
clearProxyReconnectErrorSuppression(port)
}, RECONNECT_CONFIG.STABLE_CONNECT_RESET_DELAY_MS)
} catch (error) {
console.error(`[background] Error in setPortProxy for tab ${proxyTabId}:`, error)
Expand All @@ -351,7 +406,14 @@ function isUsingOpenAICompatibleApiSession(session) {
)
}

async function executeApi(session, port, config) {
async function executeApi(
session,
port,
config,
isLatestSessionRequest = () => true,
requestGenerationId,
connectionPort = port,
) {
console.log(
`[background] executeApi called for model: ${session.modelName}, apiMode: ${session.apiMode}`,
)
Expand Down Expand Up @@ -384,37 +446,42 @@ async function executeApi(session, port, config) {
}
}
if (tabId) {
const proxyPort = connectionPort
if (!isLatestSessionRequest()) {
console.debug('[background] Skipping a superseded ChatGPT Web session request.')
return
}
console.debug(`[background] ChatGPT Tab ID ${tabId} found.`)
const hasMatchingProxy = Boolean(port.proxy && port._proxyTabId === tabId)
const hasMatchingProxy = Boolean(proxyPort.proxy && proxyPort._proxyTabId === tabId)
if (!hasMatchingProxy) {
if (port.proxy) {
if (proxyPort.proxy) {
console.debug(
`[background] Existing proxy tab ${port._proxyTabId} does not match ${tabId}; reconnecting.`,
`[background] Existing proxy tab ${proxyPort._proxyTabId} does not match ${tabId}; reconnecting.`,
)
} else {
console.debug('[background] port.proxy not found, calling setPortProxy.')
}
setPortProxy(port, tabId)
setPortProxy(proxyPort, tabId)
}
if (port.proxy && port._proxyTabId === tabId) {
if (proxyPort.proxy && proxyPort._proxyTabId === tabId) {
if (hasMatchingProxy) {
console.debug('[background] Proxy already established; forwarding session.')
}
console.debug('[background] Posting message to proxy tab:', { session: redactedSession })
try {
port.proxy.postMessage({ session })
postProxySession(proxyPort, session, requestGenerationId)
} catch (e) {
console.warn(
'[background] Error posting message to existing proxy tab in executeApi (ChatGPT Web Model):',
e,
'. Attempting to reconnect.',
{ session: redactedSession },
)
setPortProxy(port, tabId)
if (port.proxy) {
setPortProxy(proxyPort, tabId)
if (proxyPort.proxy) {
console.debug('[background] Proxy re-established. Attempting to post message again.')
try {
port.proxy.postMessage({ session })
postProxySession(proxyPort, session, requestGenerationId)
console.info('[background] Successfully posted session after proxy reconnection.')
} catch (e2) {
console.error(
Expand Down Expand Up @@ -469,18 +536,24 @@ async function executeApi(session, port, config) {
} else {
console.debug('[background] No valid ChatGPT Tab ID found. Using direct API call.')
const accessToken = await getChatGptAccessToken()
if (!isLatestSessionRequest()) {
console.debug('[background] Skipping a superseded direct ChatGPT Web session request.')
return
}
await generateAnswersWithChatgptWebApi(port, session.question, session, accessToken)
}
} else if (isUsingClaudeWebModel(session)) {
console.debug('[background] Using Claude Web Model')
const sessionKey = await getClaudeSessionKey()
if (!isLatestSessionRequest()) return
await generateAnswersWithClaudeWebApi(port, session.question, session, sessionKey)
} else if (isUsingMoonshotWebModel(session)) {
console.debug('[background] Using Moonshot Web Model')
await generateAnswersWithMoonshotWebApi(port, session.question, session, config)
} else if (isUsingBingWebModel(session)) {
console.debug('[background] Using Bing Web Model')
const accessToken = await getBingAccessToken()
if (!isLatestSessionRequest()) return
if (isUsingModelName('bingFreeSydney', session)) {
console.debug('[background] Using Bing Free Sydney model')
await generateAnswersWithBingWebApi(port, session.question, session, accessToken, true)
Expand All @@ -490,7 +563,14 @@ async function executeApi(session, port, config) {
} else if (isUsingGeminiWebModel(session)) {
console.debug('[background] Using Gemini Web Model')
const cookies = await getBardCookies()
await generateAnswersWithBardWebApi(port, session.question, session, cookies)
if (!isLatestSessionRequest()) return
await generateAnswersWithBardWebApi(
port,
session.question,
session,
cookies,
isLatestSessionRequest,
)
} else if (isUsingOpenAICompatibleApiSession(session)) {
console.debug('[background] Using OpenAI-compatible API provider')
await generateAnswersWithOpenAICompatibleApi(port, session.question, session, config)
Expand Down Expand Up @@ -935,12 +1015,21 @@ try {
}

try {
registerPortListener(async (session, port, config) => {
console.debug(
`[background] Port listener triggered for session: ${session.modelName}, port: ${port.name}`,
)
await executeApi(session, port, config)
})
registerPortListener(
async (session, port, config, isLatestSessionRequest, requestGenerationId, connectionPort) => {
console.debug(
`[background] Port listener triggered for session: ${session.modelName}, port: ${port.name}`,
)
await executeApi(
session,
port,
config,
isLatestSessionRequest,
requestGenerationId,
connectionPort,
)
},
)
console.log('[background] Port listener registered successfully.')
} catch (error) {
console.error('[background] Error registering port listener:', error)
Expand Down
64 changes: 64 additions & 0 deletions src/background/proxy-generation-state.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
export function markProxyGenerationStarted(port, proxyGenerationId, requestGenerationId) {
if (proxyGenerationId !== undefined) port._proxyGenerationId = proxyGenerationId
port._proxyRequestGenerationId = requestGenerationId
port._generating = true
port._suppressReconnectError = false
}

export function tagProxyRequestGeneration(port, message) {
return port._proxyRequestGenerationId === undefined
? message
: { ...message, requestGenerationId: port._proxyRequestGenerationId }
}

export function isCurrentProxyGenerationMessage(port, message) {
return (
message.stoppedGenerationId !== undefined ||
port._proxyGenerationId === undefined ||
(port._generating && message.proxyGenerationId === port._proxyGenerationId)
)
}

export function markProxyGenerationFinished(port) {
port._generating = false
port._suppressReconnectError = false
}

export function forwardProxyMessage(port, message) {
const forwardedMessage =
message?.stop && port._stopAcknowledged ? { ...message, stopAcknowledged: true } : message
if (message?.stop) interruptProxyGeneration(port)
port.proxy.postMessage(forwardedMessage)
}

export function markProxyGenerationFinishedFromMessage(port, message) {
if (
!isCurrentProxyGenerationMessage(port, message) ||
message.stoppedGenerationId !== undefined ||
(!message.error && !message.done)
)
return false
markProxyGenerationFinished(port)
return true
}
Comment on lines +34 to +43

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.

medium

To prevent potential runtime TypeError exceptions, add a safety check to ensure message is defined before accessing its properties. This is a good defensive programming practice.

Suggested change
export function markProxyGenerationFinishedFromMessage(port, message) {
if (message.stoppedGenerationId !== undefined || (!message.error && !message.done)) return false
markProxyGenerationFinished(port)
return true
}
export function markProxyGenerationFinishedFromMessage(port, message) {
if (!message || message.stoppedGenerationId !== undefined || (!message.error && !message.done)) return false
markProxyGenerationFinished(port)
return true
}


export function interruptProxyGeneration(port) {
if (!port._generating) return false
port._generating = false
port._suppressReconnectError = true
return true
}

export function clearProxyReconnectErrorSuppression(port) {
port._suppressReconnectError = false
}

export function consumeProxyReconnectErrorSuppression(port) {
const shouldSuppress = Boolean(port._suppressReconnectError)
port._suppressReconnectError = false
return shouldSuppress
}

export function shouldSkipProxyReconnect(port) {
return port._isClosed || Boolean(port.proxy)
}
Loading