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

fix: return Google Error when there is a missing required parameter #1291

Merged
merged 22 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/fallbackRest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@ export function encodeRequest(
if (typeof json !== 'object' || Array.isArray(json)) {
throw new Error(`Request to RPC ${rpc.name} must be an object.`);
}

const transcoded = transcode(
json,
rpc.parsedOptions,
rpc.resolvedRequestType!.fields
);

if (!transcoded) {
throw new Error(
`Cannot build HTTP request for ${JSON.stringify(json)}, method: ${
Expand Down
26 changes: 18 additions & 8 deletions src/fallbackServiceStub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,29 @@ export function generateServiceStub(
// We cannot use async-await in this function because we need to return the canceller object as soon as possible.
// Using plain old promises instead.

let fetchParameters: FetchParameters;
try {
fetchParameters = requestEncoder(
rpc,
protocol,
servicePath,
servicePort,
request
);
} catch (err) {
// we could not encode parameters; pass error to the callback
// and return a no-op canceler object.
callback(err);
return {
cancel() {},
};
}

const cancelController = hasAbortController()
? new AbortController()
: new NodeAbortController();
const cancelSignal = cancelController.signal as AbortSignal;
let cancelRequested = false;

const fetchParameters = requestEncoder(
rpc,
protocol,
servicePath,
servicePort,
request
);
const url = fetchParameters.url;
const headers = fetchParameters.headers;
for (const key of Object.keys(options)) {
Expand Down
3 changes: 2 additions & 1 deletion src/transcoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import {JSONObject, JSONValue} from 'proto3-json-serializer';
import {Field, Type} from 'protobufjs';
import {google} from '../protos/http';
import {GoogleError} from './googleError';
import {camelToSnakeCase, toCamelCase as snakeToCamelCase} from './util';

export interface TranscodedRequest {
Expand Down Expand Up @@ -340,7 +341,7 @@ export function transcode(
// all fields annotated as REQUIRED MUST be emitted in the body.
for (const requiredField of requiredFields) {
if (!(requiredField in request) || request[requiredField] === undefined) {
throw new Error(
throw new GoogleError(
`Required field ${requiredField} is not present in the request.`
);
}
Expand Down
30 changes: 21 additions & 9 deletions test/unit/transcoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
buildQueryStringComponents,
requestChangeCaseAndCleanup,
overrideHttpRules,
TranscodedRequest,
} from '../../src/transcoding';
import * as assert from 'assert';
import {
Expand Down Expand Up @@ -615,7 +616,7 @@ describe('validate proto3 field with default value', () => {
];
const transcoded = transcode(request, parsedOptions, badTestMessageFields);
assert.deepStrictEqual(
transcoded?.url,
(transcoded as TranscodedRequest)?.url,
'projects/test-project/contents/test-content'
);
});
Expand All @@ -633,7 +634,7 @@ describe('validate proto3 field with default value', () => {
];
assert.throws(
() => transcode(request, parsedOptions, testMessageFields),
Error
/Error: Required field content is not present in the request/
);
});
it('when body="*", all required field should emitted in body', () => {
Expand All @@ -650,8 +651,13 @@ describe('validate proto3 field with default value', () => {
},
];
const transcoded = transcode(request, parsedOptions, testMessageFields);
assert.deepStrictEqual(transcoded?.url, 'projects/test-project');
assert.deepStrictEqual(transcoded?.data, {content: 'test-content'});
assert.deepStrictEqual(
(transcoded as TranscodedRequest)?.url,
'projects/test-project'
);
assert.deepStrictEqual((transcoded as TranscodedRequest)?.data, {
content: 'test-content',
});
});
it('when body="*", unset optional field should remove from body', () => {
const request: RequestType = {
Expand All @@ -668,10 +674,10 @@ describe('validate proto3 field with default value', () => {
];
const transcoded = transcode(request, parsedOptions, testMessageFields);
assert.deepStrictEqual(
transcoded?.url,
(transcoded as TranscodedRequest)?.url,
'projects/test-project/contents/test-content'
);
assert.deepStrictEqual(transcoded?.data, {});
assert.deepStrictEqual((transcoded as TranscodedRequest)?.data, {});
});
it('unset optional fields should not appear in query params', () => {
const request: RequestType = {
Expand All @@ -690,9 +696,15 @@ describe('validate proto3 field with default value', () => {
},
];
const transcoded = transcode(request, parsedOptions, testMessageFields);
assert.deepStrictEqual(transcoded?.url, 'projects/test-project');
assert.deepStrictEqual(transcoded?.data, 'test-content');
assert.strictEqual(transcoded.queryString, '');
assert.deepStrictEqual(
(transcoded as TranscodedRequest)?.url,
'projects/test-project'
);
assert.deepStrictEqual(
(transcoded as TranscodedRequest)?.data,
'test-content'
);
assert.deepStrictEqual((transcoded as TranscodedRequest).queryString, '');
});
});

Expand Down