Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support overriding upstream HTTP method #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ The proxy handler object has the following properties:
* `request` - is the incoming [request object](http://hapijs.com/api#request-object). The response from this function should be an object with the following properties:
* `uri` - the absolute proxy URI.
* `headers` - optional object where each key is an HTTP request header and the value is the header content.
* `method` - optional HTTP method to use when making request to upstream server.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a word that it defaults to the same verb as the route?

* `onRequest` - a custom function which is passed the upstream request. Function signature is `function (req)` where:
* `req` - the [wreck] (https://github.com/hapijs/wreck) request to the upstream server.
* `onResponse` - a custom function for processing the response from the upstream service before sending to the client. Useful for custom error handling of responses from the proxied endpoint or other payload manipulation. Function signature is `function (err, res, request, h, settings, ttl)` where:
Expand Down
5 changes: 3 additions & 2 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ internals.handler = function (route, handlerOptions) {

return async function (request, h) {

const { uri, headers } = await settings.mapUri(request);
let { uri, headers, method } = await settings.mapUri(request);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep it that way:

Suggested change
let { uri, headers, method } = await settings.mapUri(request);
const { uri, headers, method = request.method } = await settings.mapUri(request);

Copy link
Contributor

@kanongil kanongil Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed change by @Marsup would break my existing code, that changes request.method in mapUri().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, will it break? I assumed the it fetches the request.method before calling mapUri(), but that is probably not the case, since it needs the result to know whether to fetch the default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say it's working as expected: https://runkit.com/embed/03jrtcm2e10x

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Reminder to self: Get my morning cup of coffee, before reviewing code.


const protocol = uri.split(':', 1)[0];

Expand Down Expand Up @@ -168,7 +168,8 @@ internals.handler = function (route, handlerOptions) {
downstreamStartTime = process.hrtime.bigint();
}

const promise = settings.httpClient.request(request.method, uri, options);
method = method || request.method;
kanongil marked this conversation as resolved.
Show resolved Hide resolved
const promise = settings.httpClient.request(method, uri, options);

request.events.once('disconnect', () => {

Expand Down
32 changes: 32 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,38 @@ describe('h2o2', () => {
await upstream.stop();
});

it('uses the method returned from mapUri', async () => {

const echoPutBody = function (request, h) {

return h.response(request.payload.echo + request.raw.req.headers['x-super-special']);
};

const mapUri = function (request) {

return {
uri: `http://127.0.0.1:${upstream.info.port}${request.path}${(request.url.search || '')}`,
headers: { 'x-super-special': '@' },
method: 'PUT'
};
};

const upstream = Hapi.server();
upstream.route({ method: 'PUT', path: '/echo', handler: echoPutBody });
await upstream.start();

const server = Hapi.server();
await server.register(H2o2);

server.route({ method: 'POST', path: '/echo', handler: { proxy: { mapUri } } });

const res = await server.inject({ url: '/echo', method: 'POST', payload: '{"echo":true}' });
expect(res.statusCode).to.equal(200);
expect(res.payload).to.equal('true@');

await upstream.stop();
});

it('replies with an error when it occurs in mapUri', async () => {

const mapUriWithError = function (request) {
Expand Down