On page error

[taganaka]

No description provided.

[coveralls]

Coverage increased (+0.01%) when pulling 1cf9863 on on_page_error into 637c70c on master.

[coveralls]

Coverage increased (+0.01%) when pulling 5344b07 on on_page_error into 637c70c on master.

[tmaier]

When can it be nil?
http.fetch_pages always returns an Array

Or am I overlooking something?

[taganaka]

you are right 👍

[coveralls]

Coverage increased (+0.18%) when pulling ba5876e on on_page_error into 637c70c on master.

[tmaier]

👍

[taganaka]

Not sure if at this point we should skip the call to on_page_downloaded when a page has error.
Kinda of breaking change but more consistent though

❓

[tmaier]

Well.... There is no header and no body...
This will most likely lead to errors when processing the page with xpath or other ways.
Second, the method is called on_page_downloaded. So one should expect a page downloaded.

But on the other hand, you might have code you want to run always.
And the method is not called on_page_success or on_page_sucesssfully_downloaded...

So I think you are right.
We should run on_page_downloaded regardless of the error.

I would just wish for an example which shows the possibilities of error handling with polipus.

As a side note:
I just thought about @on_before_save.each {|e| e.call(page)}
Should it run right before we store the page? And thus, skip it when storable? returns false?
My conclusion was the same. The on_before_save block is the only place where one could manipulate storable? if necessary

[taganaka]

I agree we should not change the current workflow.
on_page_error is a better semantic way to encapsulate network errors.
A better documentation plus more comprehensive examples should help the user to understand better how to proper use DSL exposed.

re on_before_save : agree to run it just before page store and after on_page_error

[coveralls]

Coverage increased (+0.18%) when pulling ad44653 on on_page_error into 637c70c on master.

[coveralls]

Coverage increased (+0.21%) when pulling 68a2e43 on on_page_error into 637c70c on master.

[coveralls]

Coverage increased (+0.21%) when pulling 68a2e43 on on_page_error into 637c70c on master.