Map freeze if mouse event below terrain

[kaij]

maplibre-gl-js version: 4.1.2

browser: Chrome 123

Steps to Trigger Behavior

1. Open https://maplibre.org/maplibre-gl-js/docs/examples/3d-terrain/
2. Rotate view with mouse such that the camera is below the mountains (see image)
3. Move mouse in white area below mountain

Link to Demonstration

https://maplibre.org/maplibre-gl-js/docs/examples/3d-terrain/

Expected Behavior

Map can be moved / dragged

Actual Behavior

Map UI freezes completely and unrecoverable. To the end user, this is like "crashing" the application.

Reason for failure

The map freezes because map.unproject throws an error if it cannot calculate coordinates below terrain. It seems this can happen quite easily with normal user interaction.

Ideas for remediations

• Catch exception in event handler? It would have no performance effect there. Also, the event could still be passed on, but with NaN coordinates

[HarelM]

I think solving the following issue will probably avoid this scenario.
Adding a try-catch probably can't hurt much, but still feeling like adding a patch and not a real solution.

• Camera can move inside 3D terrain #1542

[kaij]

I think solving the following issue will probably avoid this scenario. Adding a try-catch probably can't hurt much, but still feeling like adding a patch and not a real solution.

• Camera can move inside 3D terrain #1542

I also feel like adding exception handler feels like a hack and that it would be best to solve this at the source. However, can the camera really be kept above terrain at all times. What about an animated camera using freezeElevation=true?

If it helps, I can add these handlers in a pull request. Another solution could be to prevent the LatLng constructor from throwing errors in such cases, e.g. adding a parameter flag like silent=true at some point?

[HarelM]

Yes, there was an initial PR to do it, but it changed the map settings so it needed more work:

• Update max/min pitch in Map._render() to prevent camera collide with terrain #2759

[olsen232]

I feel that a try/catch or similar - although it may feel like you have simply hacked around the problem - is actually pretty valid here:

• try/catch was invented to let you recover from unrecoverable errors. Deciding not to catch (or suppress in some way) this error would be a good choice if this error is unrecoverable. But is this really an unrecoverable error?
• it's true, you are hoping to solve this problem at the source, and it would be good to keep the camera above the terrain at all times. But a) that's not done yet and b) wouldn't it be better if the various subsystems were robust against failure in other subsystems? Right now by having each subsystem relying on the assumption that every other subsystem is working perfectly (even though we know they are not), this is designing to encourage a cascading failure.

[HarelM]

Your points are valid, and I won't reject a PR to patch the code until we properly fix the problem. I would advise to add a comment to saying this should be removed after the issue is resolved.
We are talking about the same system, and I prefer not to write "defensive programming" if possible, and fix the source of a problem when possible. It's not always possible, I know, but I'm trying to aim high.