From f4fcf97a2744cdf16fe9665350627df89b7a3c20 Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Fri, 4 Sep 2026 19:31:47 +0200 Subject: [PATCH] Read Piped's -1 and its 200-with-an-error-body for what they are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Piped answers with "views": -1 and "uploaded": -1 wherever it has no number, and playlist rows are full of them. Mapped straight through, a row read "-0B views • 56 years ago" — -1 millisecond is a date in 1969. Every Piped-to-Video conversion now runs both fields through PipedSentinel, which reads a negative as an absent value; the epoch milliseconds the field really is stay milliseconds. Piped also refuses a write with HTTP 200 and {"error": "..."} — adding a video it cannot fetch, or removing an index that moved, both come back that way. Decoding straight into the success shape failed, and the toast showed a decoding-error string instead of the server's own sentence. All authenticated writes now read their body through decodeWriteResponse, which checks the error shape first (a body carrying error may carry message beside it) and throws APIError.serverError with the message, so the existing toasts say what actually happened. HTTPClient's status-code handling is untouched, so no other backend changes behaviour. --- Yattee/Services/API/PipedAPI.swift | 106 +++++++++++-- YatteeTests/PipedResponseTests.swift | 217 +++++++++++++++++++++++++++ 2 files changed, 307 insertions(+), 16 deletions(-) create mode 100644 YatteeTests/PipedResponseTests.swift diff --git a/Yattee/Services/API/PipedAPI.swift b/Yattee/Services/API/PipedAPI.swift index da6ddcd3..0f0711cf 100644 --- a/Yattee/Services/API/PipedAPI.swift +++ b/Yattee/Services/API/PipedAPI.swift @@ -325,7 +325,7 @@ actor PipedAPI: InstanceAPI { ) // Returns {"message": "ok"} on success - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Unsubscribes from a channel. @@ -346,7 +346,7 @@ actor PipedAPI: InstanceAPI { body: bodyData ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Fetches the user's playlists. @@ -524,7 +524,7 @@ actor PipedAPI: InstanceAPI { body: CreateRequest(name: name) ) - let response: CreateResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + let response = try await performWrite(CreateResponse.self, endpoint: endpoint, instance: instance) return response.playlistId } @@ -546,7 +546,7 @@ actor PipedAPI: InstanceAPI { body: RenameRequest(playlistId: id, newName: newName) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Replaces the description of one of the user's playlists. @@ -578,7 +578,7 @@ actor PipedAPI: InstanceAPI { body: DescriptionRequest(playlistId: id, description: description) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Deletes one of the user's playlists. @@ -597,7 +597,7 @@ actor PipedAPI: InstanceAPI { body: DeleteRequest(playlistId: id) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Appends a video to one of the user's playlists. @@ -623,7 +623,7 @@ actor PipedAPI: InstanceAPI { body: AddRequest(playlistId: id, videoIds: [videoID]) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) } /// Removes the row at a position from one of the user's playlists. @@ -650,7 +650,50 @@ actor PipedAPI: InstanceAPI { body: RemoveRequest(playlistId: id, index: index) ) - let _: PipedMessageResponse = try await httpClient.fetch(endpoint, baseURL: instance.url) + _ = try await performWrite(PipedMessageResponse.self, endpoint: endpoint, instance: instance) + } + + /// Decodes the body of an authenticated Piped write. + /// + /// Piped answers a refused write with **HTTP 200** and `{"error": "..."}`: + /// adding a video it cannot fetch — region-blocked or deleted, neither rare + /// — and removing an index that is no longer there both come back that way, + /// so the status code settles nothing here. Decoding straight into the + /// success shape turns the server's own explanation into a decoding-failure + /// message nobody can act on, so the error shape is read first: a body that + /// carries `error` may carry `message` alongside it, and checking the + /// success shape first would read right past the failure. + /// - Parameters: + /// - type: The success shape this endpoint answers with. + /// - data: The raw response body. + /// - Returns: The decoded success shape. + /// - Throws: ``APIError/serverError(_:)`` carrying the server's message when + /// the body is an error body. + static func decodeWriteResponse(_ type: T.Type, from data: Data) throws -> T { + let decoder = HTTPClient.makeAPIDecoder() + + if let failure = try? decoder.decode(PipedErrorResponse.self, from: data) { + let message = failure.error.trimmingCharacters(in: .whitespacesAndNewlines) + if !message.isEmpty { + throw APIError.serverError(message) + } + } + + do { + return try decoder.decode(type, from: data) + } catch let error as DecodingError { + throw APIError.decodingError(error) + } + } + + /// Sends an authenticated write and reads its body the way ``decodeWriteResponse(_:from:)`` does. + private func performWrite( + _ type: T.Type, + endpoint: GenericEndpoint, + instance: Instance + ) async throws -> T { + let data = try await httpClient.fetchData(endpoint, baseURL: instance.url) + return try Self.decodeWriteResponse(type, from: data) } /// Builds an authenticated JSON write request. Piped wants the token bare in @@ -677,11 +720,20 @@ private struct PipedLoginResponse: Decodable, Sendable { let token: String } -/// Generic message response from Piped API (used by subscribe/unsubscribe). -private struct PipedMessageResponse: Decodable, Sendable { +/// Generic message response from Piped API (used by subscribe/unsubscribe and +/// every playlist write). Not private so the write-decode seam can be exercised +/// against the shape the server really answers with. +struct PipedMessageResponse: Decodable, Sendable { let message: String } +/// The body Piped returns for a refused write — under HTTP 200 as often as not. +/// Some spellings carry a `message` alongside `error`, which is why this shape +/// is read before the success one. +private struct PipedErrorResponse: Decodable, Sendable { + let error: String +} + /// Subscription info from Piped API. struct PipedSubscription: Decodable, Sendable { let url: String @@ -751,6 +803,28 @@ private func stripHTML(_ html: String) -> String { return text.trimmingCharacters(in: .whitespacesAndNewlines) } +// MARK: - Piped Sentinels + +/// Piped writes `-1` where it has no value: playlist rows come back with +/// `"views": -1` and `"uploaded": -1`, and trending rows carry the same +/// sentinel. Mapped through as-is they render as "-0B views" and, once `-1` +/// milliseconds becomes a date, "56 years ago" — so a negative reading is an +/// absent one. +enum PipedSentinel { + /// A count Piped actually has, or nil for its `-1` sentinel. + static func count(_ raw: Int64?) -> Int? { + guard let raw, raw >= 0 else { return nil } + return Int(raw) + } + + /// The upload date Piped actually has, or nil for its `-1` sentinel. + /// Piped sends this field as epoch **milliseconds**. + static func uploadedDate(_ raw: Int64?) -> Date? { + guard let raw, raw >= 0 else { return nil } + return Date(timeIntervalSince1970: TimeInterval(raw) / 1000) + } +} + // MARK: - Piped Response Models private struct PipedVideo: Decodable, Sendable { @@ -787,9 +861,9 @@ private struct PipedVideo: Decodable, Sendable { thumbnailURL: uploaderAvatar.flatMap { URL(string: $0) } ), duration: TimeInterval(duration), - publishedAt: uploaded.map { Date(timeIntervalSince1970: TimeInterval($0) / 1000) }, + publishedAt: PipedSentinel.uploadedDate(uploaded), publishedText: uploadedDate, - viewCount: views.map { Int($0) }, + viewCount: PipedSentinel.count(views), likeCount: nil, thumbnails: Thumbnail.fallbackChain(for: thumbnail.flatMap { URL(string: $0) }), isLive: duration == -1, @@ -866,9 +940,9 @@ private struct PipedStreamResponse: Decodable, Sendable { thumbnailURL: uploaderAvatar.flatMap { URL(string: $0) } ), duration: TimeInterval(duration), - publishedAt: uploaded.map { Date(timeIntervalSince1970: TimeInterval($0) / 1000) }, + publishedAt: PipedSentinel.uploadedDate(uploaded), publishedText: uploadDate, - viewCount: views.map { Int($0) }, + viewCount: PipedSentinel.count(views), likeCount: likes.map { Int($0) }, thumbnails: thumbnails, isLive: livestream ?? false, @@ -1026,9 +1100,9 @@ private struct PipedSearchItem: Decodable, Sendable { thumbnailURL: uploaderAvatar.flatMap { URL(string: $0) } ), duration: TimeInterval(duration ?? 0), - publishedAt: uploaded.map { Date(timeIntervalSince1970: TimeInterval($0) / 1000) }, + publishedAt: PipedSentinel.uploadedDate(uploaded), publishedText: uploadedDate, - viewCount: views.map { Int($0) }, + viewCount: PipedSentinel.count(views), likeCount: nil, thumbnails: Thumbnail.fallbackChain(for: thumbnail.flatMap { URL(string: $0) }), isLive: duration == -1, diff --git a/YatteeTests/PipedResponseTests.swift b/YatteeTests/PipedResponseTests.swift new file mode 100644 index 00000000..a1a2e3b1 --- /dev/null +++ b/YatteeTests/PipedResponseTests.swift @@ -0,0 +1,217 @@ +// +// PipedResponseTests.swift +// YatteeTests +// +// Piped answers with two things the app used to take at face value: `-1` +// standing in for a number it does not have, and an error body served under +// HTTP 200. Both have to be read as what they are before they reach a row or +// a toast. +// + +import Testing +import Foundation +@testable import Yattee + +// MARK: - Helpers + +private let pipedBaseURL = URL(string: "https://piped.example.com")! + +/// Builds one stream row of a Piped playlist page, with whatever the server +/// claims for the two sentinel-carrying fields. +private func pipedStreamJSON( + videoId: String, + title: String, + views: Int64?, + uploaded: Int64? +) -> String { + var fields: [String] = [ + "\"url\": \"/watch?v=\(videoId)\"", + "\"title\": \"\(title)\"", + "\"uploaderName\": \"Test Channel\"", + "\"uploaderUrl\": \"/channel/UCtest\"", + "\"duration\": 120", + "\"thumbnail\": \"https://piped.example.com/thumb.jpg\"" + ] + if let views { + fields.append("\"views\": \(views)") + } + if let uploaded { + fields.append("\"uploaded\": \(uploaded)") + } + return "{\(fields.joined(separator: ", "))}" +} + +/// Builds the first page of `/playlists/{id}`. +private func pipedPlaylistPageJSON(streams: [String]) -> Data { + Data(""" + { + "name": "Watch Later", + "uploader": "arek", + "uploaderUrl": "/channel/UCowner", + "videos": \(streams.count), + "relatedStreams": [\(streams.joined(separator: ",\n"))], + "nextpage": null + } + """.utf8) +} + +// MARK: - Sentinels + +@Suite("Piped Sentinel Values") +struct PipedSentinelTests { + + @Test("A negative count is no count at all") + func negativeCountIsNil() { + #expect(PipedSentinel.count(-1) == nil) + #expect(PipedSentinel.count(-999) == nil) + } + + @Test("A count the server has is carried through, zero included") + func realCountsSurvive() { + #expect(PipedSentinel.count(0) == 0) + #expect(PipedSentinel.count(1_234_567) == 1_234_567) + } + + @Test("A missing count stays missing") + func absentCountIsNil() { + #expect(PipedSentinel.count(nil) == nil) + } + + @Test("A negative upload timestamp is not a date in 1969") + func negativeUploadedIsNil() { + #expect(PipedSentinel.uploadedDate(-1) == nil) + } + + @Test("An upload timestamp is read as epoch milliseconds") + func uploadedIsMilliseconds() { + // 2021-01-01T00:00:00Z in milliseconds. + let date = PipedSentinel.uploadedDate(1_609_459_200_000) + #expect(date == Date(timeIntervalSince1970: 1_609_459_200)) + } + + @Test("A missing upload timestamp stays missing") + func absentUploadedIsNil() { + #expect(PipedSentinel.uploadedDate(nil) == nil) + } + + @Test("A playlist row of sentinels comes back with neither views nor a date") + func playlistRowSentinelsAreDropped() throws { + let page = pipedPlaylistPageJSON(streams: [ + pipedStreamJSON(videoId: "aaa", title: "Sentinels", views: -1, uploaded: -1) + ]) + + let snapshot = try PipedAPI.userPlaylistSnapshot( + fromPageJSON: [page], + playlistID: "playlist-uuid", + instanceURL: pipedBaseURL + ) + + let video = try #require(snapshot.entries.first?.video) + #expect(video.viewCount == nil) + #expect(video.publishedAt == nil) + } + + @Test("A playlist row with real numbers keeps them, milliseconds and all") + func playlistRowRealValuesSurvive() throws { + let page = pipedPlaylistPageJSON(streams: [ + pipedStreamJSON(videoId: "bbb", title: "Real", views: 4200, uploaded: 1_609_459_200_000) + ]) + + let snapshot = try PipedAPI.userPlaylistSnapshot( + fromPageJSON: [page], + playlistID: "playlist-uuid", + instanceURL: pipedBaseURL + ) + + let video = try #require(snapshot.entries.first?.video) + #expect(video.viewCount == 4200) + #expect(video.publishedAt == Date(timeIntervalSince1970: 1_609_459_200)) + } +} + +// MARK: - Write Responses + +@Suite("Piped Write Response Decoding") +struct PipedWriteResponseTests { + + /// Stands in for the create-playlist success shape, which is declared where + /// only that one call can see it. + private struct CreateShape: Decodable { + let playlistId: String + } + + @Test("An error body under HTTP 200 is an error, not a message") + func errorBodyBecomesServerError() throws { + let data = Data(#"{"error":"Video not found"}"#.utf8) + + #expect(throws: APIError.serverError("Video not found")) { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + } + } + + @Test("The server's own words reach the toast") + func errorMessageIsReadable() { + let data = Data(#"{"error":"Index out of bounds"}"#.utf8) + + do { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + Issue.record("An error body decoded as a success") + } catch { + #expect(error.localizedDescription.contains("Index out of bounds")) + } + } + + @Test("A success body still decodes") + func successBodyDecodes() throws { + let data = Data(#"{"message":"ok"}"#.utf8) + + let response = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + #expect(response.message == "ok") + } + + @Test("An error alongside a message is still an error") + func errorWinsOverMessage() throws { + let data = Data(#"{"error":"Video not found","message":"ok"}"#.utf8) + + #expect(throws: APIError.serverError("Video not found")) { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + } + } + + @Test("A blank error is not treated as a failure") + func blankErrorFallsThroughToTheSuccessShape() throws { + let data = Data(#"{"error":" ","message":"ok"}"#.utf8) + + let response = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + #expect(response.message == "ok") + } + + @Test("The same check guards every write shape, not just the message one") + func createShapeIsGuardedToo() throws { + let failure = Data(#"{"error":"You are not authorized"}"#.utf8) + #expect(throws: APIError.serverError("You are not authorized")) { + _ = try PipedAPI.decodeWriteResponse(CreateShape.self, from: failure) + } + + let success = Data(#"{"playlistId":"abc-123"}"#.utf8) + let response = try PipedAPI.decodeWriteResponse(CreateShape.self, from: success) + #expect(response.playlistId == "abc-123") + } + + @Test("A body that is neither shape is still a decoding failure") + func unknownBodyIsADecodingError() { + let data = Data(#"{"unexpected":true}"#.utf8) + + do { + _ = try PipedAPI.decodeWriteResponse(PipedMessageResponse.self, from: data) + Issue.record("An unknown body decoded as a success") + } catch let error as APIError { + guard case .decodingError = error else { + Issue.record("Expected a decoding error, got \(error)") + return + } + } catch { + Issue.record("Expected an APIError, got \(error)") + } + } +}