Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hranicka opened this issue Jul 1, 2014 · 20 comments
Labels

Comments

@hranicka
Copy link
Contributor

hranicka commented Jul 1, 2014

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:


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.

@hrach
Copy link
Contributor

hrach commented Jul 10, 2014

@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?

@nechutny
Copy link
Contributor

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())

@mishak87
Copy link
Contributor

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.

@hrach hrach closed this as completed in 408925f Jul 10, 2014
hrach added a commit that referenced this issue Jul 10, 2014
@dg
Copy link
Member

dg commented Jul 10, 2014

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

@hrach
Copy link
Contributor

hrach commented Jul 11, 2014

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

@hrach hrach reopened this Jul 11, 2014
@hranicka
Copy link
Contributor Author

@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...

@hrach
Copy link
Contributor

hrach commented Jul 11, 2014

@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.

@hranicka
Copy link
Contributor Author

@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...

@hrach
Copy link
Contributor

hrach commented Jul 12, 2014

@hranicka thanks! :)

@hranicka
Copy link
Contributor Author

@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 :(

@hrach
Copy link
Contributor

hrach commented Jul 20, 2014

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

@hrach hrach added the 1-bug label Oct 18, 2014
@hranicka
Copy link
Contributor Author

I got it! Or no? ...

See:

  1. I have a problem with ::offsetGet:
  1. Another problem is Invalid argument supplied for foreach() here:

And maybe the core of the problem GroupedSelection!

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?

@hrach
Copy link
Contributor

hrach commented Feb 19, 2015

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

@hranicka hranicka changed the title Database/Table/ActiveRow.php:315 generates Notice: Trying to get property of non-object ActiveRow: Trying to get property of non-object & Selection: Invalid argument supplied for foreach() Feb 19, 2015
@hranicka
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@temistokles
Copy link

This (#15 (comment)) 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];
}

@dg dg closed this as completed in 3c44de6 Apr 22, 2015
@dg dg reopened this Apr 22, 2015
@hranicka
Copy link
Contributor Author

@dg I don't like 3c44de6 nowadays 😞
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.

@dg dg closed this as completed in 615803d Apr 22, 2015
@hranicka
Copy link
Contributor Author

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

@dg dg reopened this Apr 22, 2015
@temistokles
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants