Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(migration): add better error message when request fails #5567

Conversation

binoy14
Copy link
Contributor

@binoy14 binoy14 commented Jan 25, 2024

Description

Adds the actual error message from the API instead of just HTTP status code. This would be usefull in debugging issues for the end users

Before

Screenshot 2024-01-25 at 1.11.11 PM.png

After

Screenshot 2024-01-25 at 3.23.54 PM.png

What to review

Implementation makes sense

Testing

Notes for release

N/A

@binoy14 binoy14 requested a review from a team as a code owner January 25, 2024 20:27
@binoy14 binoy14 requested review from bjoerge and removed request for a team January 25, 2024 20:27
Copy link

vercel bot commented Jan 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview Jan 26, 2024 5:46pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 26, 2024 5:46pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Jan 26, 2024 5:46pm

Copy link
Contributor

github-actions bot commented Jan 25, 2024

Package Documentation Change
sanity -3%
Full Report
@sanity/migrate
This branch Next branch
1 documented 0 documented
36 not documented 0 not documented
@sanity/diff
This branch Next branch
13 documented 13 documented
16 not documented 16 not documented
@sanity/block-tools
This branch Next branch
4 documented 4 documented
9 not documented 9 not documented
@sanity/types
This branch Next branch
55 documented 55 documented
239 not documented 239 not documented
sanity/desk
This branch Next branch
84 documented 84 documented
64 not documented 64 not documented
@sanity/portable-text-editor
This branch Next branch
21 documented 21 documented
44 not documented 44 not documented
@sanity/mutator
This branch Next branch
7 documented 7 documented
4 not documented 4 not documented
@sanity/cli
This branch Next branch
1 documented 1 documented
31 not documented 31 not documented
@sanity/util/legacyDateFormat
This branch Next branch
0 documented 0 documented
5 not documented 5 not documented
@sanity/schema/_internal
This branch Next branch
0 documented 0 documented
12 not documented 12 not documented
@sanity/util/paths
This branch Next branch
1 documented 1 documented
15 not documented 15 not documented
sanity/router
This branch Next branch
17 documented 17 documented
26 not documented 26 not documented
@sanity/schema
This branch Next branch
0 documented 0 documented
2 not documented 2 not documented
sanity/structure
This branch Next branch
2 documented 2 documented
6 not documented 6 not documented
sanity/cli
This branch Next branch
2 documented 2 documented
0 not documented 0 not documented
@sanity/vision
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
@sanity/util/fs
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
sanity/_internal
This branch Next branch
0 documented 0 documented
1 not documented 1 not documented
@sanity/util/createSafeJsonParser
This branch Next branch
1 documented 0 documented
0 not documented 0 not documented
sanity/_internalBrowser
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
@sanity/util/content
This branch Next branch
1 documented 1 documented
5 not documented 5 not documented
sanity
This branch Next branch
177 documented 183 documented
843 not documented 846 not documented

Copy link
Contributor

github-actions bot commented Jan 25, 2024

Component Testing Report Updated Jan 26, 2024 5:49 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 31s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 12s 3 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 13s 4 2 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 32s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 18s 9 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 1s 18 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 6s 3 0 0

Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thank you!

if (res.status < 200 || res.status > 299) {
const err = new Error(`HTTP Error ${res.status}: ${res.statusText}`) as HTTPError
const response = await res.json()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The json parsing may fail here, should we handle this specifically?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was my first pass with try catch but since it already catches it somewhere else it broke other things but I think I can inline a catch and lot it and it should still throw the error correctly

@binoy14 binoy14 merged commit d9ce930 into feat/migration-runner Jan 26, 2024
70 of 71 checks passed
@binoy14 binoy14 deleted the 01-25-chore_migration_add_better_error_message_when_request_fails branch January 26, 2024 18:25
bjoerge pushed a commit that referenced this pull request Jan 29, 2024
* chore(migration): add better error message when request fails

* fix(migration): add error message if json parsing fails
bjoerge pushed a commit that referenced this pull request Jan 29, 2024
* chore(migration): add better error message when request fails

* fix(migration): add error message if json parsing fails
bjoerge pushed a commit that referenced this pull request Jan 29, 2024
* chore(migration): add better error message when request fails

* fix(migration): add error message if json parsing fails
bjoerge pushed a commit that referenced this pull request Jan 30, 2024
* chore(migration): add better error message when request fails

* fix(migration): add error message if json parsing fails
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2024
* feat(migrate): move into monorepo from poc

* test(migrate): make sure uint8array-extras package gets transpiled from esm

* feat(cli): scaffold create/run migration cli commands

* fix(migrate): inline functions from uint8array-extras for now

Can't be imported from package because of ESM

* feat: basic migration cli support

* feat: basic `migrations list` command

* feat: migration create command w/templates

* feat(sanity): add exports for sanity/migrate + sanity/migrate/mutations

* chore(migrate): add test script

* test(migrate): add tests for `parseJSON`

* feat(migrate): add customisable parser and iterator type to `parseJSON`

* feat(migrate): add JSON parser that can handle chunks interrupted by an error object

* refactor(migrate): rename targets => destinations

* feat(migrate): use safe JSON parser when streaming from Export HTTP API (#5542)

* refactor(util): create shared JSON parser for `@sanity/export` and `@sanity/migrate`

* feat(migrate): add safe JSON parser

* chore(util): update comment

* chore(export): update comment

* feat(migrate): use safe JSON parser when streaming from Export HTTP API

* feat(migrate): add JSON options and type parameter to NDJSON utility

* refactor(migrate): use type parameter

* fix(migrate): add missing export

* fix(util): fix package.json formatting

* feat(migrate): implement mutation batcher and use when submitting against mutate endpoint

* feat(migrate): limit request concurrency

* fix(migrate): workaround issue with p-map and ESM

* feat(cli): allow user provided concurrency

* feat(migration): improve progress as migration is running (#5550)

* feat(migration): add a prompt before runing a real migration (#5552)

### Description

<!--
What changes are introduced?
Why are these changes introduced?
What issue(s) does this solve? (with link, if possible)
-->

Adds a prompt before running a real migration with the projectId and dataset so the user is aware and intentionally selects running a real migration

### What to review

<!--
What steps should the reviewer take in order to review?
What parts/flows of the application/packages/tooling is affected?
-->

`sanity migration run <name-of-migration> --dry=false` should show a prompt like "This migration will run on the “test” dataset in “yajkdsl” project. Are you sure?"

### Testing

<!--
Did you add sufficient testing for this change?
If not, please explain how you tested this change and why it was not
possible/practical for writing an automated test
-->

### Notes for release

<!--
A description of the change(s) that should be used in the release notes.
-->

- N/A

* feat(migration): support passing an array of documentTypes to migration (#5566)

* chore(migration): add better error message when request fails (#5567)

* chore(migration): add better error message when request fails

* fix(migration): add error message if json parsing fails

* feat(migrate): add asyncIterableToStream util

* feat(migrate): add bufferThroughFile utility

* fix(migrate): buffer exports through file

* fix(cli): use dryRun from @sanity/migrate

* refactor(migrate): rename function

* refactor(migrate): add ndjson support for both parsing and stringifying in ndjson util

* refactor(sanity): remove mutiny dependency in favor of inlined creators

* fix(sanity): add --no-progress flag

* fix(migrate): simplify bufferThroughFile

* feat(migrate): unify contexts, add support for document lookup and query during a migration

* fix(dev): fix example migration script imports

* feat(migration): support async hooks in migration nodes (#5564)

* fix(sanity): use --no-dry instead of --dry=false

* feat(migrate): add support for yielding transactions

* fix(migrate): minor typing issue

* fix(sanity): polish cli and migration templates

* chore(deps): remove unused deps

* fix(sanity): fix quoting of documentTypes in migration templates

* fix(sanity): rename migration name => title

* fix(migrate): run migration against all documents if documentTypes is omitted

* fix(migration): fixes dry run not showing any information

* fix(migration): show proper error message when the file has code issues

* fix(migrate): various naming consistency fixes

* fix(sanity): add a few examples to help text for 'sanity migration create'

* fix(sanity): improve error handling on 'sanity migration run' without id

* fix(migrate): fix error asserting transaction

* fix(migrate): use transactionId instead of id on mutation payload

* feat(cli): add pretty mutation formatting to migration runner (#5573)

* feat(cli): add pretty mutation formatting to migration runner

* feat(cli): improve formatting

* feat(cli): add indent option to `prettyFormatMutation`

* feat(cli): support indentation when printing JSON

* feat(cli): support migrations for multiple document types

* feat(cli): improve dry run migration formatting

* refactor(cli): abstract tree reporter

* chore(cli): remove unused import

* fix(cli): add missing closing parenthesis

* feat(cli): add migration dry-run output

* feat(cli): underline document id when pretty formatting mutation

* feat(cli): add transaction support to pretty formatter

* feat(cli): add transaction support to pretty mutation formatter

* refactor(cli): reduce complexity

* feat(cli): pretty format mutations when committing migration

* chore(migrate): instrument with some debug logging

* refactor(migration): move filtered document methods to a separate `filtered`-context key, add a client with limited concurrency to migration context

* fix(migrate): add request tags and user agent

* fix(sanity): improve minimal example

---------

Co-authored-by: Ash <[email protected]>
Co-authored-by: Binoy Patel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants