SALTO-6867 Fetch bot builder #6751
Conversation
adfineberg
force-pushed
the
SALTO-6867-csrf-client
branch
from
9bd6ae5 to
0e08127
Compare
| } | ||
| const headers = { | ||
| ...params.headers, | ||
| 'Content-Type': 'application/json', |
There was a problem hiding this comment.
why is this always application/json ?
| } catch (e) { | ||
| if (e.response?.status === 400) { | ||
| log.debug('Got 400, trying to get new CSRF token') | ||
| this.csrfToken = await this.getCsrfToken() |
There was a problem hiding this comment.
is expired csrf token is the only reason for a 400 status?
There was a problem hiding this comment.
also, if this.getCsrfToken() fails it throws an error, but then we won't see the original error that was caught in this block
| endpoints: { | ||
| default: { | ||
| post: { | ||
| // This is for the graphql endpoint, which is readonly for fetch. It allows it to be used in fetch |
There was a problem hiding this comment.
then maybe set it specifically for the graphql path? or are there multiple?
(is there a reason to define the clients in a separate PR from the fetch definitions? there's probably no harm in adding it, but there isn't enough context here to actually review the only thing using it - even if these are in separate PRs, probably worth having the next one available for review?)
adfineberg
force-pushed
the
SALTO-6867-csrf-client
branch
from
9f709cf to
414cd9f
Compare
adfineberg
changed the title
This is still missing a filter to order the paths, some references and error handling for the graphql response
Additional context for reviewer
Release Notes:
Replace me with a short sentence that describes the effect of this change on Salto users
User Notifications:
Replace me with a short sentence that describes changes that will appear in NaCls and are not caused by user actions (e.g. a new annotation, field values that are converted to references, etc). Hidden changes should not be listed.