feat: add failOnStatusCode option to API request context #34346
Conversation
Test results for "tests 1"15 failed 0 passed Merge workflow run. |
| @@ -620,6 +620,12 @@ A list of permissions to grant to all pages in this context. See | |||
|
|
|||
| An object containing additional HTTP headers to be sent with every request. Defaults to none. | |||
|
|
|||
| ## context-option-apirequest | |||
| - `apiRequest` <[Object]> | |||
| - `failOnStatusCode` <[boolean]> | |||
There was a problem hiding this comment.
apiRequest.newContext({apiRequest: {...}}) call would look weird. Since the option will be present on both browser and API contexts, but makes sense only for the later, we can probably use a more specific name for the property failAPIRequestOnStatusCode (also not great). Also the documentation should make it clear that it will only affect API requests.
There was a problem hiding this comment.
Should I still have that option inside the apiRequest object? Or just replace the object with a more specific variable name such as failAPIRequestOnStatusCode?
There was a problem hiding this comment.
It doesn't seem to me like the option will be present on both browser and API contexts since the browser.newContext() and playwright.request.newContext() returns different types. The changes I've made so far should only affect API contexts
There was a problem hiding this comment.
Should I still have that option inside the apiRequest object? Or just replace the object with a more specific variable name such as failAPIRequestOnStatusCode?
I'd just replace it with a flat property.
It doesn't seem to me like the option will be present on both browser and API contexts since the browser.newContext() and playwright.request.newContext() returns different types.
It depends on where you add references to the new context-option-apirequest, see my comment below. I'd expect it to be present in both newContext methods mentioned above, so that it works for page.request.fetch() too. Arguably we can omit this feature in page.request and only have in global fetch, but I believe it'd be convenient if it could be set in the config.
| expect(error.message).toContain('404 Not Found'); | ||
| }); | ||
|
|
||
| it('should throw when failOnStatusCode is set to true inside the fetch API call', async ({ playwright, server }) => { |
There was a problem hiding this comment.
There is already ...should support failOnStatusCode tests that cover this scenario, no need to repeat.
|
|
||
| it('should throw when failOnStatusCode is set to true', async ({ playwright, server }) => { | ||
| it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/34204' }); | ||
| const request = await playwright.request.newContext({ apiRequest: { failOnStatusCode: true } }); |
There was a problem hiding this comment.
Let's also add a test where browser.newContext is called with this option.
| /** | ||
| * An object containing an option to throw an error when API request returns status codes other than 2xx and 3xx. | ||
| */ | ||
| apiRequest?: {failOnStatusCode?: boolean;} |
There was a problem hiding this comment.
This file is auto generated from docs/, don't edit it manually. Once you add references to context-option-apirequest in corresponding classes, this file will be automatically updated when you run build.
See context-option-extrahttpheaders for example.
| @@ -620,6 +620,12 @@ A list of permissions to grant to all pages in this context. See | |||
|
|
|||
| An object containing additional HTTP headers to be sent with every request. Defaults to none. | |||
|
|
|||
| ## context-option-apirequest | |||
| - `apiRequest` <[Object]> | |||
| - `failOnStatusCode` <[boolean]> | |||
There was a problem hiding this comment.
Should I still have that option inside the apiRequest object? Or just replace the object with a more specific variable name such as failAPIRequestOnStatusCode?
I'd just replace it with a flat property.
It doesn't seem to me like the option will be present on both browser and API contexts since the browser.newContext() and playwright.request.newContext() returns different types.
It depends on where you add references to the new context-option-apirequest, see my comment below. I'd expect it to be present in both newContext methods mentioned above, so that it works for page.request.fetch() too. Arguably we can omit this feature in page.request and only have in global fetch, but I believe it'd be convenient if it could be set in the config.
Add failOnStatusCode option to API request context so that the flag is applied to every request made unless specified.
References issue #34204