Skip to content

Commit 27d5f02

Browse files
yelizhenden-mdbyelizhenden-mdb
authored
CLOUDP-304960: Errors (APIs must return ApiError when errors occur) (#630)
1 parent de3f6cf commit 27d5f02

File tree

5 files changed

+352
-0
lines changed

5 files changed

+352
-0
lines changed

‎tools/spectral/ipa/__tests__/IPA114ErrorResponsesReferToApiError.test.js‎

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
import testRule from './__helpers__/testRule';
2+
import { DiagnosticSeverity } from '@stoplight/types';
3+
4+
const components = {
5+
responses: {
6+
badRequest: {
7+
content: {
8+
'application/json': {
9+
schema: {
10+
$ref: '#/components/schemas/ApiError',
11+
},
12+
},
13+
},
14+
description: 'Bad Request.',
15+
},
16+
notFound: {
17+
content: {
18+
'application/json': {
19+
schema: {
20+
$ref: '#/components/schemas/ApiError',
21+
},
22+
},
23+
},
24+
description: 'Not Found.',
25+
},
26+
internalServerError: {
27+
content: {
28+
'application/json': {
29+
schema: {
30+
$ref: '#/components/schemas/ApiError',
31+
},
32+
},
33+
},
34+
description: 'Internal Error.',
35+
},
36+
},
37+
schemas: {
38+
ApiError: {
39+
type: 'object',
40+
properties: {
41+
error: {
42+
type: 'string',
43+
},
44+
},
45+
},
46+
},
47+
};
48+
49+
testRule('xgen-IPA-114-error-responses-refer-to-api-error', [
50+
{
51+
name: 'valid error responses with ApiError schema',
52+
document: {
53+
paths: {
54+
'/resources': {
55+
get: {
56+
responses: {
57+
400: {
58+
$ref: '#/components/responses/badRequest',
59+
},
60+
404: {
61+
$ref: '#/components/responses/notFound',
62+
},
63+
500: {
64+
$ref: '#/components/responses/internalServerError',
65+
},
66+
},
67+
},
68+
},
69+
},
70+
components: components,
71+
},
72+
errors: [],
73+
},
74+
{
75+
name: 'invalid error responses missing schema',
76+
document: {
77+
paths: {
78+
'/resources': {
79+
get: {
80+
responses: {
81+
400: {
82+
content: {
83+
'application/json': {},
84+
},
85+
},
86+
},
87+
},
88+
},
89+
},
90+
},
91+
errors: [
92+
{
93+
code: 'xgen-IPA-114-error-responses-refer-to-api-error',
94+
message: '400 response must define a schema referencing ApiError.',
95+
path: ['paths', '/resources', 'get', 'responses', '400', 'content', 'application/json'],
96+
severity: DiagnosticSeverity.Warning,
97+
},
98+
],
99+
},
100+
{
101+
name: 'invalid error responses missing content',
102+
document: {
103+
paths: {
104+
'/resources': {
105+
get: {
106+
responses: {
107+
400: {},
108+
},
109+
},
110+
},
111+
},
112+
},
113+
errors: [
114+
{
115+
code: 'xgen-IPA-114-error-responses-refer-to-api-error',
116+
message: '400 response must define content with ApiError schema reference.',
117+
path: ['paths', '/resources', 'get', 'responses', '400'],
118+
severity: DiagnosticSeverity.Warning,
119+
},
120+
],
121+
},
122+
{
123+
name: 'invalid error responses referencing wrong schema',
124+
document: {
125+
paths: {
126+
'/resources': {
127+
get: {
128+
responses: {
129+
500: {
130+
content: {
131+
'application/json': {
132+
schema: {
133+
$ref: '#/components/schemas/Error',
134+
},
135+
},
136+
},
137+
},
138+
},
139+
},
140+
},
141+
},
142+
components: {
143+
schemas: {
144+
Error: {
145+
type: 'object',
146+
properties: {
147+
message: {
148+
type: 'string',
149+
},
150+
},
151+
},
152+
},
153+
},
154+
},
155+
errors: [
156+
{
157+
code: 'xgen-IPA-114-error-responses-refer-to-api-error',
158+
message: '500 response must reference ApiError schema.',
159+
path: ['paths', '/resources', 'get', 'responses', '500', 'content', 'application/json'],
160+
severity: DiagnosticSeverity.Warning,
161+
},
162+
],
163+
},
164+
{
165+
name: 'invalid error responses with inline schema',
166+
document: {
167+
paths: {
168+
'/resources': {
169+
get: {
170+
responses: {
171+
404: {
172+
content: {
173+
'application/json': {
174+
schema: {
175+
type: 'object',
176+
properties: {
177+
error: {
178+
type: 'string',
179+
},
180+
},
181+
},
182+
},
183+
},
184+
},
185+
},
186+
},
187+
},
188+
},
189+
},
190+
errors: [
191+
{
192+
code: 'xgen-IPA-114-error-responses-refer-to-api-error',
193+
message: '404 response must reference ApiError schema.',
194+
path: ['paths', '/resources', 'get', 'responses', '404', 'content', 'application/json'],
195+
severity: DiagnosticSeverity.Warning,
196+
},
197+
],
198+
},
199+
{
200+
name: 'error responses with exception',
201+
document: {
202+
paths: {
203+
'/resources': {
204+
get: {
205+
responses: {
206+
400: {
207+
content: {
208+
'application/json': {
209+
schema: {
210+
type: 'object',
211+
},
212+
'x-xgen-IPA-exception': {
213+
'xgen-IPA-114-error-responses-refer-to-api-error': 'Reason',
214+
},
215+
},
216+
},
217+
},
218+
500: {
219+
'x-xgen-IPA-exception': {
220+
'xgen-IPA-114-error-responses-refer-to-api-error': 'Reason',
221+
},
222+
},
223+
},
224+
},
225+
},
226+
},
227+
},
228+
errors: [],
229+
},
230+
]);

‎tools/spectral/ipa/ipa-spectral.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ extends:
1010
- ./rulesets/IPA-110.yaml
1111
- ./rulesets/IPA-112.yaml
1212
- ./rulesets/IPA-113.yaml
13+
- ./rulesets/IPA-114.yaml
1314
- ./rulesets/IPA-117.yaml
1415
- ./rulesets/IPA-123.yaml
1516
- ./rulesets/IPA-125.yaml

‎tools/spectral/ipa/rulesets/IPA-114.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# IPA-114: Errors
2+
# http://go/ipa/114
3+
4+
functions:
5+
- IPA114ErrorResponsesReferToApiError
6+
7+
rules:
8+
xgen-IPA-114-error-responses-refer-to-api-error:
9+
description: |
10+
APIs must return ApiError when errors occur
11+
12+
##### Implementation details
13+
This rule checks that all 4xx and 5xx error responses reference the ApiError schema.
14+
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-114-error-responses-refer-to-api-error'
15+
severity: warn
16+
given: '$.paths[*][*].responses[?(@property.match(/^[45]\d\d$/))]'
17+
then:
18+
function: 'IPA114ErrorResponsesReferToApiError'

‎tools/spectral/ipa/rulesets/README.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,20 @@ Rule checks for the following conditions:
603603

604604

605605

606+
### IPA-114
607+
608+
Rules are based on [http://go/ipa/IPA-114](http://go/ipa/IPA-114).
609+
610+
#### xgen-IPA-114-error-responses-refer-to-api-error
611+
612+
![warn](https://img.shields.io/badge/warning-yellow)
613+
APIs must return ApiError when errors occur
614+
615+
##### Implementation details
616+
This rule checks that all 4xx and 5xx error responses reference the ApiError schema.
617+
618+
619+
606620
### IPA-117
607621

608622
Rules are based on [http://go/ipa/IPA-117](http://go/ipa/IPA-117).

‎tools/spectral/ipa/rulesets/functions/IPA114ErrorResponsesReferToApiError.js‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { hasException } from './utils/exceptions.js';
2+
import {
3+
collectAdoption,
4+
collectAndReturnViolation,
5+
collectException,
6+
handleInternalError,
7+
} from './utils/collectionUtils.js';
8+
import { resolveObject } from './utils/componentUtils.js';
9+
import { getSchemaNameFromRef } from './utils/methodUtils.js';
10+
11+
const RULE_NAME = 'xgen-IPA-114-error-responses-refer-to-api-error';
12+
13+
/**
14+
* Verifies that 4xx and 5xx responses reference the ApiError schema
15+
*
16+
* @param {object} input - The response object to check
17+
* @param {object} _ - Rule options (unused)
18+
* @param {object} context - The context object containing path and document information
19+
*/
20+
export default (input, _, { path, documentInventory }) => {
21+
const oas = documentInventory.unresolved;
22+
const apiResponseObject = resolveObject(oas, path);
23+
const errorCode = path[path.length - 1];
24+
25+
// Check for exception at response level
26+
if (hasException(apiResponseObject, RULE_NAME)) {
27+
collectException(apiResponseObject, RULE_NAME, path);
28+
return;
29+
}
30+
31+
const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode);
32+
if (errors.length !== 0) {
33+
return collectAndReturnViolation(path, RULE_NAME, errors);
34+
}
35+
36+
collectAdoption(path, RULE_NAME);
37+
};
38+
39+
function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) {
40+
try {
41+
const errors = [];
42+
let content;
43+
44+
if (apiResponseObject.content) {
45+
content = apiResponseObject.content;
46+
} else if (apiResponseObject.$ref) {
47+
const schemaName = getSchemaNameFromRef(apiResponseObject.$ref);
48+
const responseSchema = resolveObject(oas, ['components', 'responses', schemaName]);
49+
content = responseSchema.content;
50+
} else {
51+
return [{ path, message: `${errorCode} response must define content with ApiError schema reference.` }];
52+
}
53+
54+
for (const [mediaType, mediaTypeObj] of Object.entries(content)) {
55+
if (!mediaType.endsWith('json')) {
56+
continue;
57+
}
58+
59+
if (hasException(mediaTypeObj, RULE_NAME)) {
60+
collectException(mediaTypeObj, RULE_NAME, [...path, 'content', mediaType]);
61+
continue;
62+
}
63+
64+
const contentPath = [...path, 'content', mediaType];
65+
66+
// Check if schema exists
67+
if (!mediaTypeObj.schema) {
68+
errors.push({
69+
path: contentPath,
70+
message: `${errorCode} response must define a schema referencing ApiError.`,
71+
});
72+
continue;
73+
}
74+
75+
// Check if schema references ApiError
76+
const schema = mediaTypeObj.schema;
77+
78+
if (!schema.$ref || getSchemaNameFromRef(schema.$ref) !== 'ApiError') {
79+
errors.push({
80+
path: contentPath,
81+
message: `${errorCode} response must reference ApiError schema.`,
82+
});
83+
}
84+
}
85+
return errors;
86+
} catch (e) {
87+
handleInternalError(RULE_NAME, path, e);
88+
}
89+
}

0 commit comments

Comments
 (0)