Skip to content

Serialize Error.cause when degrading storage #824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
roryabraham merged 2 commits into from
Aug 22, 2026
+25 −1
Merged

Serialize Error.cause when degrading storage #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbaa4fc
fix(storage): serialize Error.cause when degrading
roryabraham Aug 22, 2026
698a3d9
fix: flatten cause serialization and cover it
roryabraham Aug 22, 2026
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
8 changes: 7 additions & 1 deletion lib/storage/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ type Storage = {
* Degrade performance by removing the storage provider and only using cache
*/
function degradePerformance(error: Error) {
Logger.logHmmm(`Error while using ${provider.name}. Falling back to only using cache and dropping storage.\n Error: ${error.message}\n Stack: ${error.stack}\n Cause: ${error.cause}`);
let causeMessage = '';
if (error.cause instanceof Error) {
causeMessage = error.cause.message;
} else if (typeof error.cause === 'string') {
causeMessage = error.cause;
Comment on lines +24 to +27

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve non-Error cause details

When Error.cause is a valid non-string value that is not an Error instance—such as {message: 'quota denied'} or an error from another JavaScript realm—this leaves causeMessage empty, so the resulting alert contains only Cause: and loses the diagnostic information this change is intended to make readable. Safely serialize these remaining cause values or extract their message rather than discarding them.

Useful? React with 👍 / 👎.

@roryabraham roryabraham Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same class of nit as the #826 cross-realm comment.

degradePerformance only runs on our own new Error('IDBKeyVal store could not be created') from this window. We never attach a plain-object cause, and we do not run this path in an iframe that would produce a foreign-realm Error.

The point of this PR is to stop interpolating a raw cause object as [object Object]. Serializing arbitrary leftover values would reintroduce that. Error and string cover every cause we actually set.

Not changing this.

}
Logger.logHmmm(`Error while using ${provider.name}. Falling back to only using cache and dropping storage.\n Error: ${error.message}\n Stack: ${error.stack}\n Cause: ${causeMessage}`);
console.error(error);
provider = MemoryOnlyProvider;
}
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/storage/tryOrDegradePerformanceTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,24 @@ describe('storage/tryOrDegradePerformance', () => {
expect(storage.getStorageProvider().name).toBe('MemoryOnlyProvider');
});

it('serializes Error.cause as a readable string in the degrade log', async () => {
const {storage, Logger} = loadIsolatedStorage();
const capturedLogs: CapturedLog[] = [];
Logger.registerLogger((data: LogData) => capturedLogs.push({level: data.level, message: data.message}));

storage.init();

const originalProvider = storage.getStorageProvider();
const targetError = new Error('IDBKeyVal store could not be created', {cause: new Error('underlying disk is full')});
originalProvider.getAllKeys = jest.fn().mockReturnValue(Promise.reject(targetError));

await expect(storage.getAllKeys()).rejects.toBe(targetError);

const degradeLog = capturedLogs.find((log) => log.level === 'hmmm' && log.message.includes('Falling back to only using cache'));
expect(degradeLog?.message).toContain('Cause: underlying disk is full');
expect(degradeLog?.message).not.toContain('[object Object]');
});

it('propagates async rejections with unrelated messages without falling back', async () => {
const {storage, Logger} = loadIsolatedStorage();
const capturedLogs: CapturedLog[] = [];
Expand Down
Loading