mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-13 23:52:40 +00:00
fix(attribution): roll back snapshot dedup key on sync appendRecord failure
1UMh (Copilot): appendRecord can throw synchronously before returning a promise — e.g. when ensureConversationFile() rethrows a non-EEXIST writeFileSync error. The async .catch() handler attached to the promise never runs in that case, so the optimistic dedup-key set sticks on a write that never landed and permanently suppresses identical retries. Roll back lastAttributionSnapshotJson in the outer catch too. Regression test forces writeFileSync to throw EACCES on the first invocation, then asserts the second identical snapshot attempt fires a fresh write rather than getting deduped.
This commit is contained in:
parent
a53c7508d2
commit
715c258fb2
2 changed files with 46 additions and 3 deletions
|
|
@ -505,6 +505,37 @@ describe('ChatRecordingService', () => {
|
|||
afterFailure + 1,
|
||||
);
|
||||
});
|
||||
|
||||
// appendRecord can throw SYNCHRONOUSLY before returning a promise
|
||||
// (e.g. ensureConversationFile fails because the conversation
|
||||
// file can't be created). Without rollback in the outer catch,
|
||||
// the dedup key stays set on a write that never happened, so
|
||||
// all future identical snapshots get suppressed.
|
||||
it('should retry an identical snapshot after a synchronous failure', async () => {
|
||||
// First call: force writeFileSync (used by ensureConversationFile
|
||||
// to wx-create the JSONL file) to throw a non-EEXIST error.
|
||||
// ensureConversationFile rethrows that, which propagates through
|
||||
// appendRecord SYNCHRONOUSLY before any promise is returned.
|
||||
const writeFileSpy = vi.spyOn(fs, 'writeFileSync');
|
||||
writeFileSpy.mockImplementationOnce(() => {
|
||||
const e = new Error(
|
||||
'EACCES: permission denied',
|
||||
) as NodeJS.ErrnoException;
|
||||
e.code = 'EACCES';
|
||||
throw e;
|
||||
});
|
||||
|
||||
chatRecordingService.recordAttributionSnapshot(baseSnapshot);
|
||||
await chatRecordingService.flush();
|
||||
// Sync failure: writeLine never reached.
|
||||
expect(vi.mocked(jsonl.writeLine)).not.toHaveBeenCalled();
|
||||
|
||||
// Identical snapshot on retry: dedup key should have been
|
||||
// rolled back so this fires a fresh write.
|
||||
chatRecordingService.recordAttributionSnapshot(baseSnapshot);
|
||||
await chatRecordingService.flush();
|
||||
expect(vi.mocked(jsonl.writeLine)).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
// Note: Session management tests (listSessions, loadSession, deleteSession, etc.)
|
||||
|
|
|
|||
|
|
@ -1007,8 +1007,9 @@ export class ChatRecordingService {
|
|||
* than being permanently suppressed.
|
||||
*/
|
||||
recordAttributionSnapshot(snapshot: AttributionSnapshot): void {
|
||||
let json: string | undefined;
|
||||
try {
|
||||
const json = JSON.stringify(snapshot);
|
||||
json = JSON.stringify(snapshot);
|
||||
if (json === this.lastAttributionSnapshotJson) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -1021,13 +1022,24 @@ export class ChatRecordingService {
|
|||
|
||||
this.lastAttributionSnapshotJson = json;
|
||||
this.appendRecord(record).catch(() => {
|
||||
// Write failed — only roll back if the key still belongs to
|
||||
// our snapshot (a later distinct write may have overwritten it).
|
||||
// Async write failed — only roll back if the key still
|
||||
// belongs to our snapshot (a later distinct write may have
|
||||
// overwritten it).
|
||||
if (this.lastAttributionSnapshotJson === json) {
|
||||
this.lastAttributionSnapshotJson = undefined;
|
||||
}
|
||||
});
|
||||
} catch (error) {
|
||||
// appendRecord (and createBaseRecord/JSON.stringify) can throw
|
||||
// synchronously — e.g. ensureConversationFile() fails because
|
||||
// the project temp dir isn't writable. The .catch() handler
|
||||
// attached to the promise never runs in that case, so we'd
|
||||
// otherwise leave the dedup key set without a write ever
|
||||
// having landed and permanently suppress identical retries.
|
||||
// Roll back here too.
|
||||
if (json !== undefined && this.lastAttributionSnapshotJson === json) {
|
||||
this.lastAttributionSnapshotJson = undefined;
|
||||
}
|
||||
debugLogger.error('Error saving attribution snapshot:', error);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue