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

infinite loading icon and js error on maps with GFW alerts #392

Open
peacog opened this issue Dec 10, 2024 · 11 comments
Open

infinite loading icon and js error on maps with GFW alerts #392

peacog opened this issue Dec 10, 2024 · 11 comments
Assignees
Labels
Bug Something isn't working

Comments

@peacog
Copy link
Collaborator

peacog commented Dec 10, 2024

Two ajax loading icons are shown on maps with GFW alerts and one stays loading infinitely
Image

There is a javascript error in the console
Image

@peacog peacog added the Bug Something isn't working label Dec 10, 2024
@peacog peacog changed the title infinite loading icon anf js error on maps with GFW alerts infinite loading icon and js error on maps with GFW alerts Dec 10, 2024
Yash621 added a commit that referenced this issue Jan 15, 2025
@peacog peacog assigned peacog and unassigned Yash621 Jan 15, 2025
@peacog
Copy link
Collaborator Author

peacog commented Jan 15, 2025

This is caused by a bug in a farmOS patch to the itamair/geophp library. See phayes/geoPHP#115 (comment)

@peacog peacog added the On hold label Jan 15, 2025
@mstenta
Copy link
Contributor

mstenta commented Jan 15, 2025

@peacog Curious: can you help me understand how you traced this issue to the geoPHP patch?

What specifically is ending up in the JSON that causes the JavaScript syntax error in the screenshot above?

@peacog
Copy link
Collaborator Author

peacog commented Jan 15, 2025

Hi @mstenta! In my local dev environment I get a more useful error than the JS syntax error. The call to /assets/geojson/full/all (the Farm Asset GeoJSON view) fails with a 500 error and in the log I see the error

ValueError: bcmul(): Argument #2 ($num2) is not well-formed in bcmul() (line 32 of /var/www/html/vendor/itamair/geophp/lib/geometry/Polygon.class.php).

Polygon->area(1, 1) (Line: 73)
Polygon->centroid() (Line: 105)
Geometry->getCentroid() (Line: 231)
Drupal\geofield\Plugin\Field\FieldType\GeofieldItem->populateComputedValues() (Line: 217)
Drupal\geofield\Plugin\Field\FieldType\GeofieldItem->setValue(Array, ) (Line: 216)

With that I was able to add a breakpoint in the Polygon->area() method and I worked out that there are a handful of point coordinates that get converted to scientific notation when cast from float to string.

If you'd like to test it yourself I've attached one of the affected polygons.

Muzizi-river-polygon.txt

It would be nice to confirm that you get the same error.

@mstenta
Copy link
Contributor

mstenta commented Jan 15, 2025

@peacog I created a new Water asset in my local farmOS instance, and copied and pasted the contents of that file into the textarea below the map in the Location tab of /asset/add/water, then saved it. When I visit the asset view page, or the farmOS dashboard, the polygon loads successfully. So I wasn't able to replicate the error. I am testing in the farmos/farmos:3.x-dev Docker container.

I wonder if this polygon is the one that was causing your issue? Or maybe it was a different one?

@peacog
Copy link
Collaborator Author

peacog commented Jan 15, 2025

I doubled checked @mstenta and that's definitely the polygon that fails in my env. Interestingly though, the polygon renders fine on the asset page, but the map also loads an all locations layer with the /assets/geojson/full/all view, and that's where I get the error.

@mstenta
Copy link
Contributor

mstenta commented Jan 15, 2025

Hmm yea that's weird. In my environment /assets/geojson/full/all loads fine.

Screenshot from 2025-01-15 11-47-24

I only have two polygons total in this local site, though. I wonder if you have others? Maybe it's actually a different one causing the issue?

Or, maybe there's something else affecting things, like at the PHP environment configuration level. Are you testing in the farmOS Docker container? Or a different environment? Maybe there are configuration differences affecting things. 🤔

In your comment on phayes/geoPHP#115 (comment) you mentioned the number 0.00004834373416429116 specifically. Was that just a random example, or is that something you saw in your backtrace somewhere?

@peacog
Copy link
Collaborator Author

peacog commented Jan 16, 2025

@mstenta In my local env I'm using ddev, and the live site is in a docker container.
I've got 873 polygons and there are just two that have this problem. What happens is this:

  • I create an asset with a WKT polygon that contains the coordinate 0.00004834373416429116
  • It gets converted to WKB when saved to the db
  • When it's read from the db the string value is changed to scientific notation 4.8343734164291E-5, and that causes the bcmul() calls to fail.

The funny thing is, the polygon I shared with you contains 4.8343734164291E-5 but it doesn't fail for you. I think it might be a PHP version issue. I'm using PHP 8.1. A warning about exponential numbers was added in PHP 7.4 and it looks like it is now an error. See https://stackoverflow.com/a/66691695/272038
What version of PHP are you on?

@mstenta
Copy link
Contributor

mstenta commented Jan 16, 2025

The funny thing is, the polygon I shared with you contains 4.8343734164291E-5 but it doesn't fail for you.

If I search for "4.8343734164291E-5" in the polygon you sent I do not get any hits. Are you sure you sent the correct polygon?

I've tried the following, and there were no results with any:

$ grep '4.8343734164291E-5' ./Muzizi-river-polygon.txt
$ grep '4.8343734164291' ./Muzizi-river-polygon.txt
$ grep '4.834373416429' ./Muzizi-river-polygon.txt

What version of PHP are you on?

The farmOS Docker image is currently on PHP 8.3.

@mstenta
Copy link
Contributor

mstenta commented Jan 16, 2025

@peacog I wonder if you can use patches-ignore to ignore the geophp patch that farmOS applies and then apply your own. 🤔

https://www.codeenigma.com/blog/composer-patches-patches-ignore

I'm trying to think of a workaround for you, because it is unlikely that I'll have the time to investigate much deeper right now, and changing the patch that is included with farmOS core is too risky without proper thought and testing.

@peacog
Copy link
Collaborator Author

peacog commented Jan 16, 2025

@mstenta yeah, I sent the wrong polygon. This is the right one. It contains the coordinate 29.727571208467 4.8343734164291E-5

Muzizi-river.txt

Thanks very much for taking the time to test this 🙏 I'll try patches-ignore.

@peacog
Copy link
Collaborator Author

peacog commented Jan 23, 2025

Since there were only two points in asset/803 and asset/804 that had this problem we have removed the points.

@peacog peacog removed the On hold label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants