All errors are handled by a single try-catch block in ChessBrowser::newGame. If the extension encounters and unexpected error at any point, the exception bubbles up to this catch block resulting in chessbrowser-invalid-message being displayed. This message states that the specified PGN was invalid but does not give any further detail on why or how to resolve the issue causing problems for both developers and end-users. The extension should be modified to handle exceptions better and provide more useful feedback on what aspect of the PGN led to the failure.
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Improve validation of PGN | mediawiki/extensions/ChessBrowser | master | +137 -74 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T239440 Improve ChessBrowser interface | |||
| Open | None | T287990 Make ChessBrowser's user-facing error reporting more specific | |||
| Resolved | BUG REPORT | Wugapodes | T287843 Validation fails when a move is specified with both rank and file | ||
| Resolved | TheDJ | T282503 Fallback text displays even when javascript is enabled |
Event Timeline
Comment Actions
Change 719449 had a related patch set uploaded (by Wugapodes; author: Wugapodes):
[mediawiki/extensions/ChessBrowser@master] Improve validation of PGN and error reporting
Comment Actions
Change 719449 merged by jenkins-bot:
[mediawiki/extensions/ChessBrowser@master] Improve validation of PGN
Comment Actions
@TheDJ I think this was really two tickets in one. The first issue was valid PGN triggering errors with no obvious cause, and that was largely resolved by fixing the PGN validation and moving some try-catch blocks around. The second issue, still unresolved I think, is that the user-facing error reporting isn't very useful. I'm not sure how to fix that; I'd imagine it would involve a literal PGN linter in order to check for and provide various syntax warnings and errors. That feels too ambitious for where we're at in development.
It might be worth just writing that up in a new task and closing this. So it's kind of a yes to both questions :)