Skip to content

Commit

Permalink
feat(app,discovery-client): send Opentrons-Version header with HTTP…
Browse files Browse the repository at this point in the history
… API requests

Closes #6852
  • Loading branch information
mcous committed Nov 4, 2020
1 parent 8e7059a commit 0768542
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 13 deletions.
24 changes: 17 additions & 7 deletions app-shell/src/__tests__/http.test.js
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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!',
},
Expand All @@ -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/,
},
Expand Down
7 changes: 6 additions & 1 deletion app-shell/src/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -16,7 +18,10 @@ export function fetch(
input: RequestInput,
init?: RequestInit
): Promise<Response> {
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}`)
Expand Down
25 changes: 24 additions & 1 deletion app/src/robot-api/__tests__/http.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
})
})
})
2 changes: 2 additions & 0 deletions app/src/robot-api/constants.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
9 changes: 7 additions & 2 deletions app/src/robot-api/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -38,10 +40,13 @@ export function fetchRobotApi(
): Observable<RobotApiResponse> {
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
Expand Down
5 changes: 4 additions & 1 deletion discovery-client/src/__tests__/health-poller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ const fetch: JestMockFn<
$Call<typeof nodeFetch, any, any>
> = nodeFetch

const EXPECTED_FETCH_OPTS = { timeout: 10000 }
const EXPECTED_FETCH_OPTS = {
timeout: 10000,
headers: { 'Opentrons-Version': '2' },
}

const stubFetchOnce = (
stubUrl: string,
Expand Down
10 changes: 9 additions & 1 deletion discovery-client/src/health-poller.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ import type {
LogLevel,
} from './types'

const DEFAULT_REQUEST_OPTS = { timeout: 10000 }
const DEFAULT_REQUEST_OPTS = {
timeout: 10000,
// NOTE(mc, 2020-10-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-10-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
Expand Down

0 comments on commit 0768542

Please sign in to comment.