database icon indicating copy to clipboard operation
database copied to clipboard

ActiveRow: Trying to get property of non-object & Selection: Invalid argument supplied for foreach()

Open hranicka opened this issue 11 years ago • 20 comments

In some cases (delete of cache files, access to one page and then access to another page) Nette\Database\Table\ActiveRow::accessColumn generates Notice.

I can't write test case because bug is unpredictable.

My temporary solution is edit Nette\Database\Table\Selection::offsetGet, but it's only a hack IMHO (and is applied only to ::offsetGet).

public function offsetGet($key)
{
    $this->execute();

    // solves strange cache bug
    $rows = ($this->rows) ? : $this->data;

    return $rows[$key];
}

Another hack is edit Nette\Database\Table\ActiveRow::accessColumn like this:

protected function accessColumn($key, $selectColumn = TRUE)
{
    $this->table->accessColumn($key, $selectColumn);
    if ($this->table->getDataRefreshed() && !$this->dataRefreshed) {
        $row = ($this->table[$this->getSignature()]) ? : $this->table->get($this->getSignature());
        $this->data = $row->data;
        $this->dataRefreshed = TRUE;
    }
}

Related topic in forum:

  • http://forum.nette.org/cs/19508-trying-to-get-property-of-non-object

I can't reproduce issue artifically so I can't write tests and create full pull-request at the moment :( Codes above are only my "hacks" until we found right cause of the issue.

hranicka avatar Jul 01 '14 12:07 hranicka

@hranicka Sadly, I really need the reproducible usecase. On forum you have written that it appears, if .... -> so is it reproducible on your project, but you can it reproduce on sandbox? Would you accept sending privately your project with the bug to my email?

hrach avatar Jul 10 '14 07:07 hrach

I have same problem. After cleaning temp dir first page render is ok, refresh throw this error and next refresh fix it to next cleaning temp dir.

Error is throwing code:

{foreach $fb_galleries as $gal}
    <div class="album">
        <a href="{plink Gallery:facebook $gal->url}" title="{$gal->name}">
            {var $photo = $gal->related('facebook_photo')->order('id')->limit(1)->fetch()}

            {if is_object($photo)}
                {var $url = 'FB-'.$photo->url}
            {else}
                {var $url = "none"}
            {/if}
            <img src="{plink Gallery:image $url,208,150,'crop'}" alt="{$gal->name}">
            <h2>
                <strong>{$gal->name}</strong><br>{$gal->created|date:'d. m. Y'} | Fotek: {$gal->related('facebook_photo')->count()}
            </h2>
        </a>
    </div>
{/foreach}

and error is throwing {var $url = 'FB-'.$photo->url} even if it pass through if(is_object())

nechutny avatar Jul 10 '14 09:07 nechutny

I think I have encountered something similar while back. I found out that in cache was incomplete row from partial select and on full select that cache was used leaving some properties empty.

mishak87 avatar Jul 10 '14 17:07 mishak87

@hrach is finer caching granularity correct fix for this issue? Partial row should re-execute query and not to throw error.

dg avatar Jul 10 '14 23:07 dg

@dg yes, definitelly not, however, fi they weren't able to reproduci it untill now, I thing there will never be.

hrach avatar Jul 11 '14 00:07 hrach

@hrach 408925f really fixes the problem. However if I'll try reproduce the bug on nette/sandbox v2.2.2, will be it still useful for you (you've written tests for it in 408925f )?

Because our project is huge monster for this purpose, so I can try reproduce the bug on clean sandbox...

hranicka avatar Jul 11 '14 06:07 hranicka

@hranicka It would be awesome If you be able to reproduce it. No, it's not the test for that...it's just rewritten old one.

hrach avatar Jul 11 '14 09:07 hrach

@hrach I'm trying evoke the bug in simple situation in nette/sandbox v2.2.2 but there it works fine. It's really strange :( I'll try it again later with more comlex DB structure. Until I can't help more, because I can't reproduce the bug on simple project when I need... it maybe appears only in some situations on larger projects :( During week I'll try create a buggy environment in nette/sandbox, but I'm sceptic that I succeed. But hope...

hranicka avatar Jul 12 '14 09:07 hranicka

@hranicka thanks! :)

hrach avatar Jul 12 '14 09:07 hrach

@hrach So I've tried envoke the bug on nette/sandbox v2.2.2 with custom DB structures but everything worked fine. Sorry, I can't reproduce test case for the bug when I need :(

hranicka avatar Jul 20 '14 08:07 hranicka

@hranicka that's the sad story... the system is quite complex and I believe I have fiexed almost everything what I could fixed :)

hrach avatar Jul 20 '14 13:07 hrach

I got it! Or no? ...

See:

  1. I have a problem with ::offsetGet:
  • https://github.com/nette/database/blob/master/src/Database/Table/Selection.php#L963
  • it calls ::execute and works with $this->rows property - but in not predictable cases it's not an array and I get an error
  1. Another problem is Invalid argument supplied for foreach() here:
  • https://github.com/nette/database/blob/master/src/Database/Table/Selection.php#L852
  • it looks like the same problem as in ::offsetGet: $this->rows is NULL

And maybe the core of the problem GroupedSelection!

  • Selection::execute operates with $this->rows and fills also $this->data with the same values
  • but check the sound here: https://github.com/nette/database/blob/master/src/Database/Table/GroupedSelection.php#L127
  • overriden ::execute method doesn't fill $this->rows -> it sounds like the problem for me

What is the purpose of $rows and $data properties?

For the current implementation I use fix like this:

// Nette\Database\Table\Selection

    public function offsetGet($key)
    {
        $this->execute();

        // solves strange cache bug
        if ($this->rows === NULL) {
            $this->cache = NULL;
            return $this->data[$key];
        }

        return $this->rows[$key];
    }

It helps in most cases (first problem). But sometimes I get the second described error and I haven't any fix for that.

But maybe right way for the fix can be overriden ::execute method in GroupedSelection & $this->rows vs. $this->data.

I can try fix the problem, but can you please explain me differences between $this->rows and $this->data?

hranicka avatar Feb 19 '15 15:02 hranicka

One holds all fetched rows, the second holds aggregated rows (makes sence only in context of GroupedSelection)

hrach avatar Feb 19 '15 15:02 hrach

Very strange problem. I've spent several hours and I can't still reproduce the bug purposely :( Everything looks fine in Nette\Database. Maybe the problem could be in currupted cache.

I will try reproduce it maybe later again.

hranicka avatar Feb 20 '15 09:02 hranicka

For the first HTTP request the error is produced but for the next request everything works fine.

I'll try conditionally copy cached files immediatelly when error occurs and again try to find the problem. It may take some time.

hranicka avatar Feb 20 '15 09:02 hranicka

This (https://github.com/nette/database/issues/15#issuecomment-75067582) seems to have solved the issue for me:

public function offsetGet($key)
{
    $this->execute();

    // solves strange cache bug
    if ($this->rows === NULL) {
        $this->cache = NULL;
        return $this->data[$key];
    }

    return $this->rows[$key];
}

temistokles avatar Apr 22 '15 15:04 temistokles

@dg I don't like 3c44de6 nowadays :disappointed: It solves the problem only partially (::offsetGet only) and the way is ugly and very strange. I would like revert the commit 3c44de6 .

Sorry for that. But I wrote:

It helps in most cases (first problem). But sometimes I get the second described error and I haven't any fix for that.

3c44de6 solves only the ActiveRow: Notice: Trying to get property of non-object. Despite the fix I've sometimes got another problem: Selection: Invalid argument supplied for foreach().

@dg @temistokles For last few weeks I use another and more clear fix: #59

@temistokles Can you try #59 please instead of the ugly "offsetGet workaround"?

We use it on several projects where previous solution (3c44de6) wasn't enough. With #15 everything is fine now - till today without problems.

hranicka avatar Apr 22 '15 16:04 hranicka

@dg Please reopen the issue, it was closed via 615803d's commit message :smirk:

hranicka avatar Apr 22 '15 18:04 hranicka

I have applied the code as well (before trying the fix above) and it did not work by itself. Because I cannot reproduce this issue anymore at the moment, I am not sure if the patch to GroupedSelection was critical or not (anyway the problematic query does not use any grouping).

temistokles avatar Apr 23 '15 06:04 temistokles

This issue could be related to #115, because this mainly happens on heavy-traffic, not in testing environments, the multiple accesses to the "same" data could be the key.

Also I confirm the issue is still present, although not that often in my case, as before.

temistokles avatar Feb 01 '16 08:02 temistokles