Skip to content

Conversation

foodisbeast
Copy link

@foodisbeast foodisbeast commented Aug 1, 2023

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Currently RealtimeClient.push passes unsupported types to JSONSerialzation.data causing an fatal exception JSONSerialization Invalid type in JSON write (_SwiftValue). According to apple docs for JSONSerialization the only supported types are NSArray, NSDictionary, NSString, NSNumber, NSArray, NSDictionary, or NSNull, but RealtimeClient.push is passing in types ChannelEvent and ChannelTopic.

internal func push(
    topic: ChannelTopic,
    event: ChannelEvent,
    payload: Payload,
    ref: String? = nil,
    joinRef: String? = nil
  ) {
    // ... hidden for illustrative purposes
      let body: [Any?] = [joinRef, ref, topic, event, payload]
      let data = self.encode(body)
    // ... hidden for illustrative purposes
}

What is the new behavior?

Instead of passing event: ChannelEvent and topic: ChannelTopic directly, pass in their rawValue attribute since they conform to RawRepresentable

internal func push(
    topic: ChannelTopic,
    event: ChannelEvent,
    payload: Payload,
    ref: String? = nil,
    joinRef: String? = nil
  ) {
    // ... hidden for illustrative purposes
      let body: [Any?] = [joinRef, ref, topic.rawValue, event.rawValue, payload]
      let data = self.encode(body)
    // ... hidden for illustrative purposes
}

Additional Context

The reason realtime-swift throws an exception but SwiftPhoenixClient does not is because event and topic are already encoded as String types (see Socket.swift from realtime-swift master)

internal func push(topic: String,
                   event: String,
                   payload: Payload,
                   ref: String? = nil,
                   joinRef: String? = nil) {
    
    // ... hidden for illustrative purposes
      let body: [Any?] = [joinRef, ref, topic, event, payload]
      let data = self.encode(body)
    // ... hidden for illustrative purposes
}

@foodisbeast
Copy link
Author

Wondering if I should add a guard to encode @GRSouza?

guard JSONSerialization.isValidJSONObject(json) else {
  fatalError("Invalid JSON object")
}

@grdsdev
Copy link
Contributor

grdsdev commented Aug 1, 2023

@foodisbeast not sure if using the guard is better or not, as it will crash anyway. Maybe just use an assertion to catch this on debug time.

assert(JSONSerialization.isValidJSONObject(json), "Invalid JSON object")

@foodisbeast
Copy link
Author

Added assertion as you recommended @GRSouza!

@grdsdev grdsdev merged commit 43576fd into supabase-community:update-upstream Aug 1, 2023
@foodisbeast foodisbeast deleted the update-upstream-encoding-fix branch August 2, 2023 00:14
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