Skip to content

Improve handling of unrecognized documents in Connect#38015

Merged
ravicious merged 2 commits intomasterfrom
r7s/unhandled-case
Feb 12, 2024
Merged

Improve handling of unrecognized documents in Connect#38015
ravicious merged 2 commits intomasterfrom
r7s/unhandled-case

Conversation

@ravicious
Copy link
Copy Markdown
Member

#36730 made it so that Connect doesn't throw when it encounters an unknown document kind. This can typically happen when downgrading the app while your app_state.json contains documents from a newer version.

However, getResourceUri was still throwing and with "Unhandled case: [object Object]". This PR makes it so that we serialize the unhandled case to console and prevents getResourceUri from throwing.

@ravicious ravicious added backport/branch/v13 no-changelog Indicates that a PR does not require a changelog entry labels Feb 9, 2024
@ravicious ravicious requested a review from gzdunek February 9, 2024 16:33
@ravicious ravicious removed the request for review from ibeckermayer February 9, 2024 16:37
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we could use unknown instead of any?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What's the difference in this case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not significant, since we only print the field.
But if it is any, a thing like unhandledCase.abc.def is allowed - if unknown it isn't, so the type is more strict.

I will leave it up to you.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, that makes total sense for a highly "generic" field such as unhandledCase. Changed any to unknown.

@ravicious ravicious enabled auto-merge February 12, 2024 09:59
@ravicious ravicious added this pull request to the merge queue Feb 12, 2024
Merged via the queue into master with commit 17e342b Feb 12, 2024
@ravicious ravicious deleted the r7s/unhandled-case branch February 12, 2024 10:23
@public-teleport-github-review-bot
Copy link
Copy Markdown

@ravicious See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR
branch/v15 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants