also looking at this
umbrella: pet-store storefront fixes (rolling)
#3
586 files
+38082
-7487
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -374,10 +374,18 @@ export async function createErrorResponse( | ||
| 374 | 374 | handler.addQuery(query.query, query.time, query.connection) |
| 375 | 375 | } |
| 376 | 376 | |
| 377 | // Check if request wants JSON (API request) | |
| 377 | // Decide JSON vs HTML by content negotiation. Default to JSON — only | |
| 378 | // render the Ignition-style HTML page when the client explicitly asked | |
| 379 | // for HTML (top-level browser navigations send | |
| 380 | // `Accept: text/html,application/xhtml+xml,…`). Browser `fetch()` | |
| 381 | // defaults to `Accept: */*`, curl sends `Accept: */*`, and most API | |
| 382 | // clients omit the header entirely — all of those should get JSON. | |
| 383 | // The previous logic required `Accept: application/json` explicitly | |
| 384 | // AND `text/html` absent, so XHR/fetch errors leaked the dev HTML | |
| 385 | // page into JSON-consuming clients. | |
| 378 | 386 | const acceptHeader = request.headers.get('Accept') || '' |
| 379 | 387 | const isProduction = process.env.NODE_ENV === 'production' || process.env.APP_ENV === 'production' |
| 380 | if (acceptHeader.includes('application/json') && !acceptHeader.includes('text/html')) { | |
| 388 | if (!acceptHeader.includes('text/html')) { | |
| 381 | 389 | // Return JSON error for API requests. In production, strip the |
| 382 | 390 | // stack trace and recent-queries log — those leak file paths, |
| 383 | 391 | // function names, query shapes, and (potentially) parameter |