From 089dd36b6a34a87eae974d9df085609cc0701627 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Thu, 5 Nov 2020 13:56:13 -0500 Subject: [PATCH] feat(app,discovery-client): send Opentrons-Version header with HTTP requests (#6914) Closes #6852 --- app-shell/src/__tests__/http.test.js | 24 ++++++++++++------ app-shell/src/http.js | 7 +++++- app/src/robot-api/__tests__/http.test.js | 25 ++++++++++++++++++- app/src/robot-api/constants.js | 2 ++ app/src/robot-api/http.js | 9 +++++-- .../src/__tests__/health-poller.test.js | 5 +++- discovery-client/src/health-poller.js | 10 +++++++- 7 files changed, 69 insertions(+), 13 deletions(-) diff --git a/app-shell/src/__tests__/http.test.js b/app-shell/src/__tests__/http.test.js index bf31ddf56aa..f9189e4d8ba 100644 --- a/app-shell/src/__tests__/http.test.js +++ b/app-shell/src/__tests__/http.test.js @@ -1,6 +1,7 @@ import mockFetch from 'node-fetch' import isError from 'lodash/isError' +import { HTTP_API_VERSION } from '@opentrons/app/src/robot-api/constants' import * as Http from '../http' jest.mock('../config') @@ -15,21 +16,30 @@ describe('app-shell main http module', () => { { name: 'regular fetch', method: Http.fetch, - request: 'http://exmple.com', + request: 'http://example.com', + requestOptions: { + headers: { 'Opentrons-Version': `${HTTP_API_VERSION}` }, + }, response: { ok: true }, expected: { ok: true }, }, { name: 'fetchJson parses json', method: Http.fetchJson, - request: 'http://exmple.com', + request: 'http://example.com', + requestOptions: { + headers: { 'Opentrons-Version': `${HTTP_API_VERSION}` }, + }, response: { ok: true, json: () => Promise.resolve({ json: 'json' }) }, expected: { json: 'json' }, }, { name: 'fetchText parses text', method: Http.fetchText, - request: 'http://exmple.com', + request: 'http://example.com', + requestOptions: { + headers: { 'Opentrons-Version': `${HTTP_API_VERSION}` }, + }, response: { ok: true, text: () => Promise.resolve('text!') }, expected: 'text!', }, @@ -39,28 +49,28 @@ describe('app-shell main http module', () => { { name: 'regular fetch fails', method: Http.fetch, - request: 'http://exmple.com', + request: 'http://example.com', response: new Error('Failed to fetch'), expected: /Failed to fetch/, }, { name: 'regular fetch returns non-success', method: Http.fetch, - request: 'http://exmple.com', + request: 'http://example.com', response: { ok: false, status: 500, statusText: 'Internal Server Error' }, expected: /Request error: 500 - Internal Server Error/, }, { name: 'fetchJson fails to parse', method: Http.fetchJson, - request: 'http://exmple.com', + request: 'http://example.com', response: { ok: true, json: () => Promise.reject(new Error('BAD')) }, expected: /BAD/, }, { name: 'fetchText fails to parse text', method: Http.fetchText, - request: 'http://exmple.com', + request: 'http://example.com', response: { ok: true, text: () => Promise.reject(new Error('BAD')) }, expected: /BAD/, }, diff --git a/app-shell/src/http.js b/app-shell/src/http.js index a9ed8347b5b..a82269eca08 100644 --- a/app-shell/src/http.js +++ b/app-shell/src/http.js @@ -6,6 +6,8 @@ import pump from 'pump' import _fetch from 'node-fetch' import FormData from 'form-data' +import { HTTP_API_VERSION } from '@opentrons/app/src/robot-api/constants' + import type { Request, RequestInit, Response } from 'node-fetch' type RequestInput = Request | string @@ -16,7 +18,10 @@ export function fetch( input: RequestInput, init?: RequestInit ): Promise { - return _fetch(input, init).then(response => { + const opts = init ?? {} + opts.headers = { ...opts.headers, 'Opentrons-Version': `${HTTP_API_VERSION}` } + + return _fetch(input, opts).then(response => { if (!response.ok) { const error = `${response.status} - ${response.statusText}` throw new Error(`Request error: ${error}`) diff --git a/app/src/robot-api/__tests__/http.test.js b/app/src/robot-api/__tests__/http.test.js index 95684ae9f09..6b185233f73 100644 --- a/app/src/robot-api/__tests__/http.test.js +++ b/app/src/robot-api/__tests__/http.test.js @@ -9,7 +9,7 @@ import fetch from 'node-fetch' import FormData from 'form-data' import { robotApiUrl, fetchRobotApi } from '../http' -import { GET, POST, PATCH, DELETE } from '../constants' +import { HTTP_API_VERSION, GET, POST, PATCH, DELETE } from '../constants' import type { $Application } from 'express' import type { RobotHost } from '../types' @@ -237,4 +237,27 @@ describe('robot-api http client', () => { ok: true, }) }) + + it('adds the Opentrons-Version header', () => { + testApp.get('/version', (req, res) => { + const version: any = req.header('Opentrons-Version') + res.status(200).send(`{ "version": "${version}" }`) + }) + + const result = fetchRobotApi(robot, { + method: GET, + path: '/version', + }).toPromise() + + expect(HTTP_API_VERSION).toEqual(expect.any(Number)) + + return expect(result).resolves.toEqual({ + host: robot, + method: GET, + path: '/version', + body: { version: `${HTTP_API_VERSION}` }, + status: 200, + ok: true, + }) + }) }) diff --git a/app/src/robot-api/constants.js b/app/src/robot-api/constants.js index b9cae8ac3c6..15cfdd81195 100644 --- a/app/src/robot-api/constants.js +++ b/app/src/robot-api/constants.js @@ -1,5 +1,7 @@ // @flow +export const HTTP_API_VERSION: 2 = 2 + export const GET: 'GET' = 'GET' export const POST: 'POST' = 'POST' export const PUT: 'PUT' = 'PUT' diff --git a/app/src/robot-api/http.js b/app/src/robot-api/http.js index 9f0c032646f..d69d47a9364 100644 --- a/app/src/robot-api/http.js +++ b/app/src/robot-api/http.js @@ -6,6 +6,8 @@ import mapValues from 'lodash/mapValues' import toString from 'lodash/toString' import omitBy from 'lodash/omitBy' +import { HTTP_API_VERSION } from './constants' + import type { Observable } from 'rxjs' import type { RobotHost, @@ -38,10 +40,13 @@ export function fetchRobotApi( ): Observable { const { path, method, body: reqBody, form: reqForm } = request const url = robotApiUrl(host, request) - const options: RequestOptions = { method } + const options: RequestOptions = { + method, + headers: { 'Opentrons-Version': `${HTTP_API_VERSION}` }, + } if (reqBody != null) { - options.headers = { 'Content-Type': 'application/json' } + options.headers = { ...options.headers, 'Content-Type': 'application/json' } options.body = JSON.stringify(reqBody) } else if (reqForm != null) { options.body = reqForm diff --git a/discovery-client/src/__tests__/health-poller.test.js b/discovery-client/src/__tests__/health-poller.test.js index ce442575b78..e65b27741fb 100644 --- a/discovery-client/src/__tests__/health-poller.test.js +++ b/discovery-client/src/__tests__/health-poller.test.js @@ -15,7 +15,10 @@ const fetch: JestMockFn< $Call > = nodeFetch -const EXPECTED_FETCH_OPTS = { timeout: 10000 } +const EXPECTED_FETCH_OPTS = { + timeout: 10000, + headers: { 'Opentrons-Version': '2' }, +} const stubFetchOnce = ( stubUrl: string, diff --git a/discovery-client/src/health-poller.js b/discovery-client/src/health-poller.js index df0bd3ce435..1a256c9d3a6 100644 --- a/discovery-client/src/health-poller.js +++ b/discovery-client/src/health-poller.js @@ -21,7 +21,15 @@ import type { LogLevel, } from './types' -const DEFAULT_REQUEST_OPTS = { timeout: 10000 } +const DEFAULT_REQUEST_OPTS = { + timeout: 10000, + // NOTE(mc, 2020-11-04): This api version is slightly duplicated + // across the larger monorepo codebase and is a good argument for a + // standalone API client library that app, app-shell, and DC can share + // NOTE(mc, 2020-11-04): Discovery client should remain locked to the lowest + // available HTTP API version that satisfies its data needs + headers: { 'Opentrons-Version': '2' }, +} /** * Create a HealthPoller to monitor the health of a set of IP addresses