chore(compass-main): refactor auto-updater to return objects when no update is available #6849
Conversation
| const json = await response.json(); | ||
| assert( | ||
| typeof json === 'object' && json !== null, | ||
| 'Expected response to be an object' | ||
| ); | ||
| assert('name' in json, 'Expected "name" in response'); | ||
| assert('to' in json, 'Expected "to" in response'); | ||
| assert('from' in json, 'Expected "from" in response'); | ||
|
|
||
| const { name, from, to } = json; | ||
| assert(typeof name === 'string', 'Expected "name" to be a string'); | ||
| assert(typeof from === 'string', 'Expected "from" to be a string'); | ||
| assert(typeof to === 'string', 'Expected "to" to be a string'); | ||
|
|
||
| return { available: true, name, from, to }; |
There was a problem hiding this comment.
I'm including this more defensive parsing of the response from the server.
kraenhansen
added
no release notes
| } | ||
|
|
||
|
|
||
| assert(response.ok); |
There was a problem hiding this comment.
Are we changing the backend to return more success status codes than 200 (EDIT: meaning with actual update data and not empty)? I don't think we expect anything else but 200 as a success state, not sure why we're making the check less strict here
There was a problem hiding this comment.
I'll revert the change and return early instead 👍 (it was a leftover change from the codeI'm introducing in #6850 - where I needed the check to be less strict, but I later moved this assert below my new code).
gribnoysup
left a comment
gribnoysup
left a comment
There was a problem hiding this comment.
Not sure why we're allowing more responses now, but maybe that's expected. Otherwise looks good
gribnoysup
changed the title
Description
As a preparation for a PR adding specific reasons for auto updates being unavailable I'm refactoring the auto updater to return an object with
{ available: false }instead ofnullwhen an update is not available.Checklist
Motivation and Context
Open Questions
Dependents
Types of changes