Skip to content

Commit 1df15ce

Browse files
CLOUDP-304960: Errors (APIs must return ApiError when errors occur)
1 parent 044ed0a commit 1df15ce

File tree

5 files changed

+318
-0
lines changed

5 files changed

+318
-0
lines changed
Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
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 referencing wrong schema',
102+
document: {
103+
paths: {
104+
'/resources': {
105+
get: {
106+
responses: {
107+
500: {
108+
content: {
109+
'application/json': {
110+
schema: {
111+
$ref: '#/components/schemas/Error',
112+
},
113+
},
114+
},
115+
},
116+
},
117+
},
118+
},
119+
},
120+
components: {
121+
schemas: {
122+
Error: {
123+
type: 'object',
124+
properties: {
125+
message: {
126+
type: 'string',
127+
},
128+
},
129+
},
130+
},
131+
},
132+
},
133+
errors: [
134+
{
135+
code: 'xgen-IPA-114-error-responses-refer-to-api-error',
136+
message: '500 response must reference ApiError schema',
137+
path: ['paths', '/resources', 'get', 'responses', '500', 'content', 'application/json'],
138+
severity: DiagnosticSeverity.Warning,
139+
},
140+
],
141+
},
142+
{
143+
name: 'invalid error responses with inline schema',
144+
document: {
145+
paths: {
146+
'/resources': {
147+
get: {
148+
responses: {
149+
404: {
150+
content: {
151+
'application/json': {
152+
schema: {
153+
type: 'object',
154+
properties: {
155+
error: {
156+
type: 'string',
157+
},
158+
},
159+
},
160+
},
161+
},
162+
},
163+
},
164+
},
165+
},
166+
},
167+
},
168+
errors: [
169+
{
170+
code: 'xgen-IPA-114-error-responses-refer-to-api-error',
171+
message: '404 response must reference ApiError schema',
172+
path: ['paths', '/resources', 'get', 'responses', '404', 'content', 'application/json'],
173+
severity: DiagnosticSeverity.Warning,
174+
},
175+
],
176+
},
177+
{
178+
name: 'error responses with exception',
179+
document: {
180+
paths: {
181+
'/resources': {
182+
get: {
183+
responses: {
184+
400: {
185+
content: {
186+
'application/json': {
187+
schema: {
188+
type: 'object',
189+
},
190+
'x-xgen-IPA-exception': {
191+
'xgen-IPA-114-error-responses-refer-to-api-error': 'Reason',
192+
},
193+
},
194+
},
195+
},
196+
},
197+
},
198+
},
199+
},
200+
},
201+
errors: [],
202+
},
203+
]);

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
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+
given: '$.paths[*][*].responses[?(@property.match(/^[45]\d\d$/))]'
16+
severity: warn
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).
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
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 {{ path: string[], documentInventory: object}} context - The context object containing the path and document
19+
* @returns {object|void} - Violation object if any errors found, otherwise undefined
20+
*/
21+
export default (input, _, { path, documentInventory }) => {
22+
const oas = documentInventory.unresolved;
23+
const apiResponseObject = resolveObject(oas, path);
24+
const errorCode = path[path.length - 1]; // e.g., "400", "404", "500"
25+
26+
const errors = checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode);
27+
28+
if (errors.length !== 0) {
29+
return collectAndReturnViolation(path, RULE_NAME, errors);
30+
}
31+
32+
collectAdoption(path, RULE_NAME);
33+
};
34+
35+
function checkViolationsAndReturnErrors(apiResponseObject, oas, path, errorCode) {
36+
try {
37+
const errors = [];
38+
let content;
39+
40+
if (apiResponseObject.content) {
41+
content = apiResponseObject.content;
42+
} else if (apiResponseObject.$ref) {
43+
const schemaName = getSchemaNameFromRef(apiResponseObject.$ref);
44+
const responseSchema = resolveObject(oas, ['components', 'responses', schemaName]);
45+
content = responseSchema.content;
46+
}
47+
48+
for (const [mediaType, mediaTypeObj] of Object.entries(content)) {
49+
if (!mediaType.endsWith('json')) {
50+
continue;
51+
}
52+
53+
if (hasException(mediaTypeObj, RULE_NAME)) {
54+
collectException(mediaTypeObj, RULE_NAME, [...path, 'content', mediaType]);
55+
continue;
56+
}
57+
58+
const contentPath = [...path, 'content', mediaType];
59+
60+
// Check if schema exists
61+
if (!mediaTypeObj || !mediaTypeObj.schema) {
62+
errors.push({
63+
path: contentPath,
64+
message: `${errorCode} response must define a schema referencing ApiError`,
65+
});
66+
continue;
67+
}
68+
69+
// Check if schema references ApiError
70+
const schema = mediaTypeObj.schema;
71+
if (!schema.$ref || !schema.$ref.endsWith('/ApiError')) {
72+
errors.push({
73+
path: contentPath,
74+
message: `${errorCode} response must reference ApiError schema`,
75+
});
76+
}
77+
}
78+
return errors;
79+
} catch (e) {
80+
handleInternalError(RULE_NAME, path, e);
81+
}
82+
}

0 commit comments

Comments
 (0)