fermoya / SwiftUIPager

Native Pager in SwiftUI
MIT License
1.27k stars 168 forks source link

[BUG] Crash when deleting last item in 2.4.0 #279

Closed sdavidliu closed 2 years ago

sdavidliu commented 2 years ago

Describe the bug

Swift/ContiguousArrayBuffer.swift:575: Fatal error: Index out of range 2022-05-16 12:42:16.914884-0700 TestSwiftUIApp[6474:134889] Swift/ContiguousArrayBuffer.swift:575: Fatal error: Index out of range

Screen Shot 2022-05-16 at 12 42 34 PM

To Reproduce Only reproducible in SwiftUIPager 2.4.0 and 2.3.3. Sample code below. Scroll to the very end and click on the "Delete" button.

struct EmbeddedExampleView: View {
    @StateObject var page1: Page = .first()
    @State var data = Array(0..<3)

    @State var alignment: ExamplePositionAlignment = .start

    var body: some View {
        NavigationView {
            GeometryReader { proxy in
                ScrollView {
                    VStack {
                        Pager(page: self.page1,
                              data: self.data,
                              id: \.self) { page in
                                self.pageView(page)
                        }
                        .itemSpacing(10)
                        .itemAspectRatio(0.8, alignment: .end)
                        .padding(8)
                        .frame(width: min(proxy.size.height, proxy.size.width),
                               height: min(proxy.size.height, proxy.size.width))
                        .background(Color.gray.opacity(0.2))

                        // New code below
                        Spacer()
                        Text("Scroll to the end and press Delete")
                        Button(action: {
                            data.remove(at: data.count - 1)
                        }, label: {
                            Text("Delete")
                                .font(.largeTitle)
                                .foregroundColor(.red)
                        })
                    }
                }
            }.navigationBarTitle("Inside a Scrollview", displayMode: .inline)
        }.navigationViewStyle(StackNavigationViewStyle())
    }

    func pageView(_ page: Int) -> some View {
        ZStack {
            Rectangle()
                .fill(Color.yellow)
            Text("Page: \(page)")
                .bold()
        }
        .cornerRadius(5)
        .shadow(radius: 5)
    }
}

Expected behavior Safely handle crash or maybe automatically scroll to the last available index. Does not crash in 2.3.2.

Environment: OSX: iOS 15.4 Device: Any Simulator or Device SwiftUIPager version: 2.4.0 or 2.3.3

fermoya commented 2 years ago

should be done in 2.5.0-beta.1, do you mind giving me some feedback @sdavidliu please?

MrConnorKenway commented 2 years ago

I encountered the same crash as sdavidliu a few hours ago, and tried to update to 2.5.0-beta.1 just now, but this time I got a divided by zero exception in line

_index = (newValue + totalPages) % totalPages

Any thoughts?

fermoya commented 2 years ago

@MrConnorKenway are you deleting the last page? Can you give me more info?

MrConnorKenway commented 2 years ago

@fermoya I didn't delete the last page. Here are the partial call stack when the fatal error 'division by zero in remainder operation' occurs:

#6      0x0000000109ea2143 in Page.index.setter at .../SourcePackages/checkouts/SwiftUIPager/Sources/SwiftUIPager/Page.swift:32                                        
#7      0x0000000109ea23eb in Page.totalPages.didset at .../SourcePackages/checkouts/SwiftUIPager/Sources/SwiftUIPager/Page.swift:40
#8      0x0000000109ea24a7 in Page.totalPages.setter ()
#9      0x0000000109ed5694 in Pager.PagerContent.loopPages(_:repeating:) at .../SourcePackages/checkouts/SwiftUIPager/Sources/SwiftUIPager/PagerContent+Buildable.swift:56
#10     0x0000000109ec9b45 in Pager.content(for:) at .../SourcePackages/checkouts/SwiftUIPager/Sources/SwiftUIPager/Pager.swift:204
#11     0x0000000109ec8aeb in closure #1 in Pager.body.getter at .../SourcePackages/checkouts/SwiftUIPager/Sources/SwiftUIPager/Pager.swift:191

and the code that uses Pager:

    var body: some View {
        Pager(page: page, data: galleries) { 
            // omitted closure
        }
        .preferredItemSize(Defaults.FrameSize.cardCellSize)
        .interactive(opacity: 0.2).itemSpacing(20)
        .loopPages().pagingPriority(.high)
        .synchronize($pageIndex, $page.index)
        .frame(height: Defaults.FrameSize.cardCellHeight)
    }
MrConnorKenway commented 2 years ago

@fermoya I've figured out the problem. My app first inits Pager with self.pagerModel.totalPages = data.count and data.count turns out to be 0. 669d20f0ab90dc6cac5aadbb8462505875805459 triggers self.index = index before my code updating totalPages to non-zero value and thus crashing my app. Perhaps we need some sanity checks in the constructor of Page to prevent unintended zero input?

PierreCapo commented 2 years ago

Having the exact same issue sometimes on 2.4.0.

I am installing right now 2.5.0-beta.1. @fermoya I'll let you know how it goes with it. Many thanks for your time and this great library 🙏

PierreCapo commented 2 years ago

Looks like I haven't got any crash so far, on both development and my users, so I think 2.5.0-beta.1 has solved the problem @fermoya

ChristianSlanzi commented 2 years ago

@fermoya Having the same issue 100% of times on 2.4.0., got fixed with 2.5.0-beta.1, any plan for a release? Or an 2.4.1 hot-fix?

fermoya commented 2 years ago

Yes, will do a release next week. I'd like to take some time and review other bugs/feature requests first

sisoje commented 7 months ago

Now for us 2.5.0 crashes and 2.4.0 is not crashing - in the same scenario. There is a ticket open for 6 months: https://github.com/fermoya/SwiftUIPager/issues/319