From 0bc0a610beea9fbe0b5b0c6c6f90736139617d67 Mon Sep 17 00:00:00 2001 From: Philipp Wagner Date: Sun, 8 Sep 2024 18:30:04 +0200 Subject: [PATCH] Properly swallow errors if deleting a remote fails (#505) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `deleteRemote()` intends to return successfully if the to-be-deleted remote does not exist. In this case, git returns an exit code of 2, and outputs a message to stderr. Depending on the locale of the user's system, this error message might be localized. `spawnPromise()` and `spawnStream()` try to set the locale to `en_US`, but there are no guarantees that this locale is actually available on the user's system. Instead of doing fragile locale-dependent string parsing, simply use the exit code we get from git in this case and act on that. Fix the mocks in the tests to behave like the real-world git, and add a test for the non-English case as well. Co-authored-by: Søren Louv-Jansen --- src/lib/git.test.ts | 18 +++++++++++++++--- src/lib/git.ts | 9 ++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/lib/git.test.ts b/src/lib/git.test.ts index 3d89f835..7acafd2e 100644 --- a/src/lib/git.test.ts +++ b/src/lib/git.test.ts @@ -364,12 +364,24 @@ describe('deleteRemote', () => { repoName: 'kibana', } as ValidConfigOptions; - it('should swallow SpawnError error', async () => { + it('should swallow "no such remote" error', async () => { const err = new childProcess.SpawnError({ - code: 128, + code: 2, + cmdArgs: [], + stdout: '', + stderr: "fatal: No such remote: 'my-remote'\n", + }); + + jest.spyOn(childProcess, 'spawnPromise').mockRejectedValueOnce(err); + await expect(await deleteRemote(options, remoteName)).toBe(undefined); + }); + + it('should swallow "no such remote" error, even if it is not in English', async () => { + const err = new childProcess.SpawnError({ + code: 2, cmdArgs: [], stdout: '', - stderr: "fatal: No such remote: 'origin'\n", + stderr: "Fehler: Remote-Repository nicht gefunden: 'my-remote'\n", }); jest.spyOn(childProcess, 'spawnPromise').mockRejectedValueOnce(err); diff --git a/src/lib/git.ts b/src/lib/git.ts index 43e9052c..93932a60 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -191,11 +191,10 @@ export async function deleteRemote( } catch (e) { const isSpawnError = e instanceof SpawnError; - if ( - isSpawnError && - e.context.code > 0 && - e.context.stderr.includes('No such remote') - ) { + // Swallow the "remote does not exist" failure, indicated by a return + // code of 2. From `git help remote`: "When subcommands such as add, rename, + // and remove can’t find the remote in question, the exit status is 2." + if (isSpawnError && e.context.code == 2) { return; }