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

Realtime types (e.g. RealtimeHistoryParams) extending Rest ones (e.g. RestHistoryParams) can cause problems (crashes) #241

Closed
ben-xD opened this issue Nov 19, 2021 · 0 comments · Fixed by #327
Assignees
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@ben-xD
Copy link
Contributor

ben-xD commented Nov 19, 2021

Realtime types (e.g. {{RealtimeHistoryParams}}) extending Rest ones (e.g. {{RestHistoryParams}}) can cause problems (crashes)

Using

{code}
_rest!.channels.get(defaultChannel).history(
ably.RealtimeHistoryParams(
limit: 10,
untilAttach: true,
),
),
{code}
Will crash the app with

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'ARTRealtimeHistoryQuery used from outside ARTRealtimeChannel.history'

Notice that {{RealtimeHistoryParams}} is passed as an argument to a {{RestChannel.history}}. Because in Ably Flutter, {{RealtimeHistoryParams}} is a {{RestHistoryParams}}, the user is allowed to do this. We should stop having {{RealtimeHistoryParams}} being a {{RestHistoryParams}}.

There are other instances of this behaviour, where a realtime class extends a rest class because it shares some code. We should consider cases like this and object oriented programming, and find a solution to it.

@ben-xD ben-xD added the bug Something isn't working. It's clear that this does need to be fixed. label Nov 19, 2021
@ben-xD ben-xD self-assigned this Nov 19, 2021
@ben-xD ben-xD changed the title Fix types Fix inheritance Nov 19, 2021
@ben-xD ben-xD changed the title Fix inheritance Realtime types (e.g. RealtimeHistoryParams) extending Rest ones (e.g. RestHistoryParams) can cause problems (crashes) Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

Successfully merging a pull request may close this issue.

2 participants