Skip to content

Commit 761377f

Browse files
mcmiremcmire
committed
Use error causes to wrap errors
1 parent e367ac1 commit 761377f

File tree

8 files changed

+137
-150
lines changed

8 files changed

+137
-150
lines changed

‎package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"debug": "^4.3.4",
2828
"execa": "^5.0.0",
2929
"glob": "^8.0.3",
30+
"pony-cause": "^2.1.0",
3031
"semver": "^7.3.7",
3132
"which": "^2.0.2",
3233
"yaml": "^2.1.1",

‎src/fs.test.ts

Lines changed: 41 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,16 @@ describe('fs', () => {
3131
});
3232
});
3333

34-
it('re-throws any error that occurs, assigning it the same code, a wrapped message, and a new stack', async () => {
34+
it('re-throws any error that occurs as a new error that points to the original', async () => {
3535
await withSandbox(async (sandbox) => {
3636
const filePath = path.join(sandbox.directoryPath, 'nonexistent');
3737

3838
await expect(readFile(filePath)).rejects.toThrow(
3939
expect.objectContaining({
40-
message: expect.stringMatching(
41-
new RegExp(
42-
`^Could not read file '${filePath}': ENOENT: no such file or directory, open '${filePath}'`,
43-
'u',
44-
),
45-
),
46-
code: 'ENOENT',
47-
stack: expect.anything(),
40+
message: `Could not read file '${filePath}'`,
41+
cause: expect.objectContaining({
42+
message: `ENOENT: no such file or directory, open '${filePath}'`,
43+
}),
4844
}),
4945
);
5046
});
@@ -64,21 +60,17 @@ describe('fs', () => {
6460
});
6561
});
6662

67-
it('re-throws any error that occurs, assigning it the same code, a wrapped message, and a new stack', async () => {
63+
it('re-throws any error that occurs as a new error that points to the original', async () => {
6864
await withSandbox(async (sandbox) => {
6965
await promisifiedRimraf(sandbox.directoryPath);
7066
const filePath = path.join(sandbox.directoryPath, 'test');
7167

7268
await expect(writeFile(filePath, 'some content 😄')).rejects.toThrow(
7369
expect.objectContaining({
74-
message: expect.stringMatching(
75-
new RegExp(
76-
`^Could not write file '${filePath}': ENOENT: no such file or directory, open '${filePath}'`,
77-
'u',
78-
),
79-
),
80-
code: 'ENOENT',
81-
stack: expect.anything(),
70+
message: `Could not write file '${filePath}'`,
71+
cause: expect.objectContaining({
72+
message: `ENOENT: no such file or directory, open '${filePath}'`,
73+
}),
8274
}),
8375
);
8476
});
@@ -97,20 +89,17 @@ describe('fs', () => {
9789
});
9890
});
9991

100-
it('re-throws any error that occurs, assigning it the same code, a wrapped message, and a new stack', async () => {
92+
it('re-throws any error that occurs as a new error that points to the original', async () => {
10193
const filePath = '/some/file';
102-
const error: any = new Error('oops');
103-
error.code = 'ESOMETHING';
104-
error.stack = 'some stack';
94+
const error = new Error('oops');
10595
when(jest.spyOn(actionUtils, 'readJsonObjectFile'))
10696
.calledWith(filePath)
10797
.mockRejectedValue(error);
10898

10999
await expect(readJsonObjectFile(filePath)).rejects.toThrow(
110100
expect.objectContaining({
111-
message: `Could not read JSON file '${filePath}': oops`,
112-
code: 'ESOMETHING',
113-
stack: 'some stack',
101+
message: `Could not read JSON file '${filePath}'`,
102+
cause: error,
114103
}),
115104
);
116105
});
@@ -126,20 +115,17 @@ describe('fs', () => {
126115
expect(await writeJsonFile(filePath, { some: 'object' })).toBeUndefined();
127116
});
128117

129-
it('re-throws any error that occurs, assigning it the same code, a wrapped message, and a new stack', async () => {
118+
it('re-throws any error that occurs as a new error that points to the original', async () => {
130119
const filePath = '/some/file';
131-
const error: any = new Error('oops');
132-
error.code = 'ESOMETHING';
133-
error.stack = 'some stack';
120+
const error = new Error('oops');
134121
when(jest.spyOn(actionUtils, 'writeJsonFile'))
135122
.calledWith(filePath, { some: 'object' })
136123
.mockRejectedValue(error);
137124

138125
await expect(writeJsonFile(filePath, { some: 'object' })).rejects.toThrow(
139126
expect.objectContaining({
140-
message: `Could not write JSON file '${filePath}': oops`,
141-
code: 'ESOMETHING',
142-
stack: 'some stack',
127+
message: `Could not write JSON file '${filePath}'`,
128+
cause: error,
143129
}),
144130
);
145131
});
@@ -183,9 +169,23 @@ describe('fs', () => {
183169

184170
await expect(fileExists(entryPath)).rejects.toThrow(
185171
expect.objectContaining({
186-
message: `Could not determine if file exists '${entryPath}': oops`,
187-
code: 'ESOMETHING',
188-
stack: 'some stack',
172+
message: `Could not determine if file exists '${entryPath}'`,
173+
cause: error,
174+
}),
175+
);
176+
});
177+
178+
it('re-throws any error that occurs as a new error that points to the original', async () => {
179+
const entryPath = '/some/file';
180+
const error = new Error('oops');
181+
when(jest.spyOn(fs.promises, 'stat'))
182+
.calledWith(entryPath)
183+
.mockRejectedValue(error);
184+
185+
await expect(fileExists(entryPath)).rejects.toThrow(
186+
expect.objectContaining({
187+
message: `Could not determine if file exists '${entryPath}'`,
188+
cause: error,
189189
}),
190190
);
191191
});
@@ -237,18 +237,15 @@ describe('fs', () => {
237237

238238
it('re-throws any error that occurs, assigning it the same code, a wrapped message, and a new stack', async () => {
239239
const directoryPath = '/some/directory';
240-
const error: any = new Error('oops');
241-
error.code = 'ESOMETHING';
242-
error.stack = 'some stack';
240+
const error = new Error('oops');
243241
when(jest.spyOn(fs.promises, 'mkdir'))
244242
.calledWith(directoryPath, { recursive: true })
245243
.mockRejectedValue(error);
246244

247245
await expect(ensureDirectoryPathExists(directoryPath)).rejects.toThrow(
248246
expect.objectContaining({
249-
message: `Could not create directory path '${directoryPath}': oops`,
250-
code: 'ESOMETHING',
251-
stack: 'some stack',
247+
message: `Could not create directory path '${directoryPath}'`,
248+
cause: error,
252249
}),
253250
);
254251
});
@@ -273,18 +270,15 @@ describe('fs', () => {
273270

274271
it('re-throws any error that occurs, assigning it the same code, a wrapped message, and a new stack', async () => {
275272
const filePath = '/some/file';
276-
const error: any = new Error('oops');
277-
error.code = 'ESOMETHING';
278-
error.stack = 'some stack';
273+
const error = new Error('oops');
279274
when(jest.spyOn(fs.promises, 'rm'))
280275
.calledWith(filePath, { force: true })
281276
.mockRejectedValue(error);
282277

283278
await expect(removeFile(filePath)).rejects.toThrow(
284279
expect.objectContaining({
285-
message: `Could not remove file '${filePath}': oops`,
286-
code: 'ESOMETHING',
287-
stack: 'some stack',
280+
message: `Could not remove file '${filePath}'`,
281+
cause: error,
288282
}),
289283
);
290284
});

‎src/fs.ts

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
readJsonObjectFile as underlyingReadJsonObjectFile,
44
writeJsonFile as underlyingWriteJsonFile,
55
} from '@metamask/action-utils';
6-
import { wrapError, isErrorWithCode } from './misc-utils';
6+
import { coverError, isErrorWithCode } from './misc-utils';
77

88
/**
99
* Reads the file at the given path, assuming its content is encoded as UTF-8.
@@ -16,10 +16,7 @@ export async function readFile(filePath: string): Promise<string> {
1616
try {
1717
return await fs.promises.readFile(filePath, 'utf8');
1818
} catch (error) {
19-
throw wrapError(
20-
error,
21-
({ message }) => `Could not read file '${filePath}': ${message}`,
22-
);
19+
throw coverError(`Could not read file '${filePath}'`, error);
2320
}
2421
}
2522

@@ -37,10 +34,7 @@ export async function writeFile(
3734
try {
3835
await fs.promises.writeFile(filePath, content);
3936
} catch (error) {
40-
throw wrapError(
41-
error,
42-
({ message }) => `Could not write file '${filePath}': ${message}`,
43-
);
37+
throw coverError(`Could not write file '${filePath}'`, error);
4438
}
4539
}
4640

@@ -62,10 +56,7 @@ export async function readJsonObjectFile(
6256
try {
6357
return await underlyingReadJsonObjectFile(filePath);
6458
} catch (error) {
65-
throw wrapError(
66-
error,
67-
({ message }) => `Could not read JSON file '${filePath}': ${message}`,
68-
);
59+
throw coverError(`Could not read JSON file '${filePath}'`, error);
6960
}
7061
}
7162

@@ -86,10 +77,7 @@ export async function writeJsonFile(
8677
try {
8778
await underlyingWriteJsonFile(filePath, jsonValue);
8879
} catch (error) {
89-
throw wrapError(
90-
error,
91-
({ message }) => `Could not write JSON file '${filePath}': ${message}`,
92-
);
80+
throw coverError(`Could not write JSON file '${filePath}'`, error);
9381
}
9482
}
9583

@@ -109,10 +97,9 @@ export async function fileExists(entryPath: string): Promise<boolean> {
10997
return false;
11098
}
11199

112-
throw wrapError(
100+
throw coverError(
101+
`Could not determine if file exists '${entryPath}'`,
113102
error,
114-
({ message }) =>
115-
`Could not determine if file exists '${entryPath}': ${message}`,
116103
);
117104
}
118105
}
@@ -131,10 +118,9 @@ export async function ensureDirectoryPathExists(
131118
try {
132119
return await fs.promises.mkdir(directoryPath, { recursive: true });
133120
} catch (error) {
134-
throw wrapError(
121+
throw coverError(
122+
`Could not create directory path '${directoryPath}'`,
135123
error,
136-
({ message }) =>
137-
`Could not create directory path '${directoryPath}': ${message}`,
138124
);
139125
}
140126
}
@@ -150,9 +136,6 @@ export async function removeFile(filePath: string): Promise<void> {
150136
try {
151137
return await fs.promises.rm(filePath, { force: true });
152138
} catch (error) {
153-
throw wrapError(
154-
error,
155-
({ message }) => `Could not remove file '${filePath}': ${message}`,
156-
);
139+
throw coverError(`Could not remove file '${filePath}'`, error);
157140
}
158141
}

‎src/misc-utils.test.ts

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
isErrorWithCode,
55
isErrorWithMessage,
66
isErrorWithStack,
7-
wrapError,
7+
coverError,
88
resolveExecutable,
99
getStdoutFromCommand,
1010
runCommand,
@@ -80,41 +80,28 @@ describe('misc-utils', () => {
8080
});
8181
});
8282

83-
describe('wrapError', () => {
84-
it('wraps the given error object by prepending the given prefix to its message', () => {
85-
const error = new Error('Some message');
83+
describe('coverError', () => {
84+
it('returns a new Error that links to the given Error', () => {
85+
const originalError = new Error('oops');
86+
const newError = coverError('Some message', originalError);
8687

87-
expect(
88-
wrapError(error, ({ message }) => `Some prefix: ${message}`),
89-
).toMatchObject({
90-
message: 'Some prefix: Some message',
91-
});
88+
expect(newError.message).toStrictEqual('Some message');
89+
expect(newError.cause).toBe(originalError);
9290
});
9391

94-
it('returns a new error object that retains the "code" property of the original error object', () => {
95-
const error: any = new Error('foo');
96-
error.code = 'ESOMETHING';
92+
it('copies over any "code" property that exists on the given Error', () => {
93+
const originalError: any = new Error('oops');
94+
originalError.code = 'CODE';
95+
const newError: any = coverError('Some message', originalError);
9796

98-
expect(wrapError(error)).toMatchObject({
99-
code: 'ESOMETHING',
100-
});
97+
expect(newError.code).toStrictEqual('CODE');
10198
});
10299

103-
it('returns a new error object that retains the "stack" property of the original error object', () => {
104-
const error: any = new Error('foo');
105-
error.stack = 'some stack';
106-
107-
expect(wrapError(error)).toMatchObject({
108-
stack: 'some stack',
109-
});
110-
});
100+
it('returns a new Error which prefixes the given message', () => {
101+
const newError = coverError('Some message', 'Some original message');
111102

112-
it('wraps the given string by prepending the given prefix to it', () => {
113-
expect(
114-
wrapError('Some message', ({ message }) => `Some prefix: ${message}`),
115-
).toMatchObject({
116-
message: 'Some prefix: Some message',
117-
});
103+
expect(newError.message).toBe('Some message: Some original message');
104+
expect(newError.cause).toBeUndefined();
118105
});
119106
});
120107

0 commit comments

Comments
 (0)