No data found for task x, cannot append received data

[RolandasRazma]

One of our tests hitting assert in URLSessionClient.swift:230 in on CI after updating to latest version (0.31.0).
It's not networking stack test so don't think we doing anything specific to hit that assert. Looking at code that would indicate some kind race condition...

[designatednerd]

Weird! Is it possible it's something leaking over from an older test?

[RolandasRazma]

can't see anything relevant, also it don't happen every run

[RolandasRazma]

public func invalidate() {
    self.clearAllTasks()
    self.session?.invalidateAndCancel()
    self.session = nil
  }

shouldn't self.session?.invalidateAndCancel() be before self.clearAllTasks()? Will try change and see if our tests passes more reliably

[designatednerd]

Hi! Yeah flipping the cleanup order likely wouldn't help since some tasks are still in-flight, I suspect I need to actually cancel all the in-flight tasks. Thanks for the heads up on this!

[designatednerd]

Haha damn it, looks like there's a key piece of info about invalidateAndCancel that's only in the docs docs and not in the headerdoc:

[designatednerd]

Shipped with version 0.32.1!

[philfi]

Hi @designatednerd,

I am encountering this issue in 0.33.0, looking at #1383 I'm not quite certain it addressed the root issue given the shared URLSession isn't being used in URLSessionClient? I have a fairly reproducible case where this invalidate logic is getting hit but I'm still encountering this assertion error afterwards, confirmed with some debugging its definitely the same instance floating around.

That all said, it does seem that the issue is resolved by calling invalidateAndCancel prior to calling the clear block (as is currently done in the else branch). I've tested this quite a bit and have been unable to trigger the assertion failure anymore. I'll go ahead and open a PR for you to take a look, hopefully it helps!

[RolandasRazma]

Can confirm that it's not solved in 0.33.0. Just got assert while running tests on iOS12

[RolandasRazma]

still happening in 0.36.0 (iOS12 simulator)

[alkincakiralar1996]

same crash on me at the latest version.

IOS 14.2 iPhone 12 how can i fix this ?

[designatednerd]

This should not be crashing in versions above 0.37.0 since it's checking to see if the session is still around before it tries to perform an operation - @alkincakiralar1996 can you confirm exactly which version you're using?

[alkincakiralar1996]

Ah sorry yes i was using older version, i update and problem resolved thanks

[m1entus]

It is crashing for me for Apollo 0.38.3 - maybe because of cancelling the task?

[designatednerd]

That's certainly possible, but since this is an assertionFailure rather than a fatalError, it will only crash in development and not in production.

[m1entus]

Yeah sure, but if we are deploing dev version to testers they have this kind of crash :< btw i think potential solution is that cancel, should cancel the task first and then remove it from tasks inside:

  open func cancel(task: URLSessionTask) {
    self.clear(task: task.taskIdentifier)
    task.cancel()
  }

[designatednerd]

🤔 My recollection is we tried to do it the opposite way before but it was causing way more problems because the task would get cancelled before it was removed from the list of tasks, but I'd be curious to hear if flipping it solves your problem?

[m1entus]

I was only guessing, this is very random for me unfortunately :/

[m1entus]

btw. what about checking if dataTask state == .cancelled and then asserting if not?

[designatednerd]

I'm not sure how that would help - the assertion exists right now in the function that adds data to the task, but it's checking to ensure a task is actually in the dictionary before doing anything. Adding an assert for something that was cancelled would still cause your same problem.

Honestly, I'd strongly recommend using TestFlight (which IIRC uses builds signed for release) rather than sending out builds in dev mode. You'll get a much, much more accurate reflection of real-world performance and you'll also avoid problems like this since assertions are designed to no-op when in release mode.

[m1entus]

But if you wil llook on line URLSessionClient:198 // No completion blocks, the task has likely been cancelled. Bail out. you already saing that if task is cancelled then would be removed from tasks. so anyway in func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) you should append additional data only if guard dataTask.state != .cancelling else { return } - then assertion would make sense for me, because you would checking wrong state assering it. In that case you are expecting that if task is cancelled it will not have data in task taking URLSessionClient:198 comment, and still you are doing assertion and crashing application.

I am not talking only about testers, me as a developer also develop app in dev environment so it is also crashing for me sometimes because i am cancelling some tasks.

In summary - i jsut think that assertions make sense if you expect that app is wrong state - in that case all is good and still we have assertion.

[designatednerd]

The comment you're referring to is in didCompleteWithError rather than didReceive data.

It's reasonable to expect something that's been cancelled to call into a method that's around completion. In theory if the task has been cancelled, it should no longer be calling back into didReceive data. I'd be curious to hear if you throw a breakpoint in at the assertion failure what the state of the task is at that point.

[m1entus]

Yes this was exacly what i did - i triggered breakpoint or actually assertion triggers and state was == 2 so it was cancelled - thats why i wrote about this in last comment.

[designatednerd]

Great, that's super-helpful to know, I can make some changes tomorrow that take this into account. Thank you!

[m1entus]

Cool thanks!

[designatednerd]

@m1entus #1677