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

[ECO-4874] Replaced all occurences of checking error message #1957

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

maratal
Copy link
Collaborator

@maratal maratal commented Aug 11, 2024

Replaced all occurences of checking error message instead of error code (or removed where the error code check was already presented) except places where error is locally generated (since it doesn't contain proper error code - 0 usually).

Closes #1946

@maratal maratal requested a review from umair-ably August 11, 2024 13:10
@maratal maratal changed the title [ECO-4874] Replaced all occurences of checking error message instead of error co… [ECO-4874] Replaced all occurences of checking error message Aug 11, 2024
@github-actions github-actions bot temporarily deployed to staging/pull/1957/features August 11, 2024 13:10 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1957/jazzydoc August 11, 2024 13:13 Inactive
@lawrence-forooghian
Copy link
Collaborator

To check I’ve understood — the error messages that you've removed the checks for are all ones that are generated by the server?

@maratal
Copy link
Collaborator Author

maratal commented Aug 14, 2024

To check I’ve understood — the error messages that you've removed the checks for are all ones that are generated by the server?

correct

Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

Hey @maratal, I just ran into an issue with ably-cocoa tests checking against realtime error messages and the assertion which failed was this one

expect(error.localizedDescription).to(contain("Invalid accessToken"))
, would you mind fixing that in this PR too?

@owenpearson
Copy link
Member

@maratal found some more here as well:

@maratal
Copy link
Collaborator Author

maratal commented Aug 14, 2024

Thanks @owenpearson I was looking for error.message but there are also description, localizedDescription, reason.message and error?.message! 🤦‍♂️

…de (or removed where the error code check was already presented) except places where error is locally generated (since it doesn't contain proper error code - 0 usually).
Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@maratal maratal merged commit 85326a6 into main Aug 15, 2024
7 checks passed
@maratal maratal deleted the fix/1946-replace-error-message-with-code branch August 15, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Test fails due to changed error message
3 participants