fix: commands break inelegantly without internet connection (#352)

* wip: handle undefined response

* wip: handle get version tags error

* wip: messaging and exit code for list remote

* wip: move process exit to command action

* test: update logic

* test: util functions

* test: list remote command

* fix: missing Makefile node call

Makefile

@@ -56,7 +56,7 @@ help:
 
 .PHONY: install
 install: build-production
-	@USE_LOCAL=true scripts/install.js
+	@USE_LOCAL=true node scripts/install.js
 
 .PHONY: install-watch
 install-watch: node_modules

src/commands/listRemote.js

@@ -12,13 +12,19 @@ export const listRemote = async () => {
         const [remoteVersions, versionInUse, localVersions] = await Promise.all(
             [getVersionsFromTags(), getVersionInUse(), getYarnVersions()],
         )
+        if (!remoteVersions.length) {
+            throw new Error('No versions available for install')
+        }
         await printVersions({
             list: remoteVersions,
             message: 'Versions available for install:',
             versionInUse,
             localVersions,
         })
+        return 0
     } catch (e) {
-        log.error(e)
+        log(e.message)
+        log.info(e.stack)
+        return 1
     }
 }

src/util/utils.js

@@ -27,12 +27,13 @@ export const getRequest = memoize(async url => {
     }
     return new Promise((resolve, reject) => {
         request.get(options, (error, response, body) => {
-            if (error || response.statusCode !== 200) {
-                if (response.body) {
+            const { statusCode, body: responseBody } = response || {}
+            if (error || statusCode !== 200) {
+                if (responseBody) {
                     if (error) {
                         log(error)
                     }
-                    return reject(response.body)
+                    return reject(responseBody)
                 }
                 return reject(error)
             }
@@ -57,5 +58,12 @@ export const getReleasesFromTags = memoize(async () => {
 })
 
 export const getVersionsFromTags = memoize(async () => {
-    return Object.keys(await getReleasesFromTags())
+    try {
+        return Object.keys(await getReleasesFromTags())
+    } catch (e) {
+        log.error(
+            'Unable to retrieve remote versions. Please check your network connection',
+        )
+        return []
+    }
 })

src/util/version.js

@@ -137,11 +137,10 @@ export const getSplitVersionAndArgs = async (maybeVersionArg, ...rest) => {
     let versionToUse
     try {
         if (maybeVersionArg) {
-            log.info(`Attempting to resolve ${maybeVersionArg}.`)
+            log.info(`Attempting to resolve ${maybeVersionArg}`)
             const parsedVersionString = await resolveVersion({
                 versionString: maybeVersionArg,
             }).catch(e => {
-                log.info(e.message)
                 log.info(e.stack)
             })
             if (parsedVersionString) {

src/yvm.js

@@ -103,7 +103,12 @@ argParser
     .action(async () => {
         log.info('Checking for installed yarn versions...')
         const { listVersions } = await import('./commands/list')
-        listVersions()
+        try {
+            await listVersions()
+        } catch (e) {
+            log(e.message)
+            process.exit(2)
+        }
     })
 
 argParser
@@ -113,7 +118,12 @@ argParser
     .action(async () => {
         log.info('Checking for available yarn versions...')
         const { listRemote } = await import('./commands/listRemote')
-        listRemote()
+        try {
+            process.exit(await listRemote())
+        } catch (e) {
+            log(e.message)
+            process.exit(2)
+        }
     })
 
 argParser.command('alias [<pattern>]', 'Show all aliases matching <pattern>')

test/commands/listRemote.test.js

@@ -13,9 +13,7 @@ jest.mock('../../src/util/version', () => {
         getYarnVersions: jest.fn().mockImplementation(() => {
             return MOCK_LOCAL_VERSIONS
         }),
-        printVersions: jest.fn().mockImplementation(() => {
-            throw new Error('mock error')
-        }),
+        printVersions: jest.fn(),
     }
 })
 jest.mock('../../src/util/utils', () => {
@@ -28,8 +26,14 @@ jest.mock('../../src/util/utils', () => {
 })
 
 describe('yvm list-remote', () => {
+    beforeAll(() => {
+        jest.spyOn(log, 'default')
+    })
+    beforeEach(jest.clearAllMocks)
+    afterAll(jest.restoreAllMocks)
+
     it('lists the response from the API', async () => {
-        await listRemote()
+        expect(await listRemote()).toEqual(0)
         expect(getVersionInUse).toHaveBeenCalled()
         expect(getVersionsFromTags).toHaveBeenCalled()
         const callArgs = printVersions.mock.calls[0][0]
@@ -39,13 +43,16 @@ describe('yvm list-remote', () => {
     })
 
     it('prints error if any fail', async () => {
-        jest.spyOn(log, 'error')
-        await listRemote()
-        expect(log.error).toHaveBeenCalledWith(new Error('mock error'))
-        log.error.mockRestore()
+        printVersions.mockRejectedValue(new Error('mock error'))
+        expect(await listRemote()).toEqual(1)
+        expect(log.default).toHaveBeenCalledWith('mock error')
     })
 
-    afterAll(() => {
-        jest.clearMocks()
+    it('prints error when no remote version available', async () => {
+        getVersionsFromTags.mockResolvedValue([])
+        expect(await listRemote()).toEqual(1)
+        expect(log.default).toHaveBeenCalledWith(
+            expect.stringContaining('No versions available'),
+        )
     })
 })

test/util/__snapshots__/utils.test.js.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Util functions Highlights all installed versions 1`] = `
+exports[`printVersions Highlights all installed versions 1`] = `
 Array [
   Array [
     "highlight installed versions",
@@ -23,7 +23,7 @@ Array [
 ]
 `;
 
-exports[`Util functions Highlights the default version 1`] = `
+exports[`printVersions Highlights the default version 1`] = `
 Array [
   Array [
     "highlight default version",
@@ -46,7 +46,7 @@ Array [
 ]
 `;
 
-exports[`Util functions Highlights the version currently in use 1`] = `
+exports[`printVersions Highlights the version currently in use 1`] = `
 Array [
   Array [
     "highlight current version",
@@ -69,7 +69,7 @@ Array [
 ]
 `;
 
-exports[`Util functions Prints all versions passed to printVersion function 1`] = `
+exports[`printVersions Prints all versions passed to printVersion function 1`] = `
 Array [
   Array [
     "print all versions",

test/util/utils.test.js

@@ -1,23 +1,165 @@
+import request from 'request'
+
 import log from '../../src/util/log'
 import {
     DEFAULT_VERSION_TEXT,
     VERSION_IN_USE_SYMBOL,
     VERSION_INSTALLED_SYMBOL,
     printVersions,
 } from '../../src/util/version'
+import {
+    getReleasesFromTags,
+    getRequest,
+    getVersionsFromTags,
+} from '../../src/util/utils'
 
 const versionInUse = '1.2.0'
 const defaultVersion = '1.3.0'
 const localVersions = ['1.1.0', versionInUse, defaultVersion]
 const versions = [...localVersions, '1.4.0', '1.5.0']
 
-describe('Util functions', () => {
+beforeAll(() => {
     jest.spyOn(log, 'default')
+})
+beforeEach(jest.clearAllMocks)
+afterAll(jest.restoreAllMocks)
+
+describe('getRequest', () => {
+    beforeAll(() => {
+        jest.spyOn(request, 'get')
+    })
+    afterAll(() => {
+        request.get.mockRestore()
+    })
+
+    it('calls request with correct options', async () => {
+        const url = 'https://some.url'
+        await getRequest(url).catch(() => {})
+        expect(request.get).toHaveBeenCalledWith(
+            expect.objectContaining({
+                url,
+                gzip: true,
+                headers: {
+                    'User-Agent': 'YVM',
+                },
+            }),
+            expect.any(Function),
+        )
+    })
+
+    it('resolves with body if there is no error', async () => {
+        const mockResponseBody = 'mock body'
+        request.get.mockImplementationOnce((_, callback) => {
+            callback(null, { statusCode: 200 }, mockResponseBody)
+        })
+        expect(await getRequest('https://any.url')).toEqual(mockResponseBody)
+    })
+
+    it('rejects with error if there is one', async () => {
+        const mockError = new Error('mock error')
+        request.get.mockImplementationOnce((_, callback) => {
+            callback(mockError, null, null)
+        })
+        try {
+            await getRequest('https://invalid.url')
+        } catch (rejection) {
+            expect(rejection).toBe(mockError)
+        }
+    })
+
+    it('rejects with response body', async () => {
+        const mockResponseBody = 'mock response body'
+        request.get.mockImplementationOnce((_, callback) => {
+            callback(null, { statusCode: 400, body: mockResponseBody })
+        })
+        try {
+            await getRequest('https://wrong.url')
+        } catch (rejection) {
+            expect(log.default).not.toHaveBeenCalled()
+            expect(rejection).toBe(mockResponseBody)
+        }
+    })
+
+    it('rejects with response body and logs error', async () => {
+        const mockError = new Error('mock error')
+        const mockResponseBody = 'mock response body'
+        request.get.mockImplementationOnce((_, callback) => {
+            callback(mockError, { statusCode: 400, body: mockResponseBody })
+        })
+        try {
+            await getRequest('https://wrong.invalid.url')
+        } catch (rejection) {
+            expect(log.default).toHaveBeenCalledWith(mockError)
+            expect(rejection).toBe(mockResponseBody)
+        }
+    })
+})
+
+describe('getVersionsFromTags', () => {
+    beforeAll(() => {
+        jest.spyOn(log, 'error')
+        jest.spyOn(request, 'get')
+    })
+    beforeEach(() => {
+        getRequest.cache.clear()
+        getReleasesFromTags.cache.clear()
+        getVersionsFromTags.cache.clear()
+    })
+
+    it('gets version from tags', async () => {
+        request.get.mockImplementationOnce((_, callback) => {
+            const body = [
+                {
+                    name: 'v1.15.2',
+                    zipball_url:
+                        'https://api.github.com/repos/yarnpkg/yarn/zipball/v1.15.2',
+                    tarball_url:
+                        'https://api.github.com/repos/yarnpkg/yarn/tarball/v1.15.2',
+                    commit: {
+                        sha: '6065f7ac8f7cec2b2c35094788117be0deae2f37',
+                        url:
+                            'https://api.github.com/repos/yarnpkg/yarn/commits/6065f7ac8f7cec2b2c35094788117be0deae2f37',
+                    },
+                    node_id: 'MDM6UmVmNDk5NzA2NDI6djEuMTUuMg==',
+                },
+                {
+                    name: 'v0.9.8',
+                    zipball_url:
+                        'https://api.github.com/repos/yarnpkg/yarn/zipball/v0.9.8',
+                    tarball_url:
+                        'https://api.github.com/repos/yarnpkg/yarn/tarball/v0.9.8',
+                    commit: {
+                        sha: 'mocksha',
+                        url:
+                            'https://api.github.com/repos/yarnpkg/yarn/commits/mockcommit',
+                    },
+                    node_id: 'mocknodeid==',
+                },
+            ]
+            callback(null, { statusCode: 200 }, JSON.stringify(body))
+        })
+        expect(await getVersionsFromTags()).toEqual(['1.15.2'])
+    })
+
+    it('logs error and returns empty on failure', async () => {
+        const mockError = new Error('mock error')
+        request.get.mockImplementationOnce((_, callback) => {
+            callback(mockError)
+        })
+        expect(await getVersionsFromTags()).toEqual([])
+        expect(log.error).toHaveBeenCalledWith(
+            expect.stringContaining('Unable to retrieve remote versions'),
+        )
+        expect(log.error).toHaveBeenCalledWith(
+            expect.stringContaining('Please check your network connection'),
+        )
+    })
+})
+
+describe('printVersions', () => {
     afterEach(() => {
         jest.resetAllMocks()
     })
-    afterAll(jest.restoreAllMocks)
-
     it('Prints all versions passed to printVersion function', async () => {
         const versionsObject = await printVersions({
             message: 'print all versions',